Task.Run lambda is taking wrong local variable from loop

Question

Hi in the following example,

void Main()
{
    Func<int, int> getPage = (pageNumber) =>
    {
        SimulateSlowHttpRequest();
        Console.WriteLine(pageNumber);
        return pageNumber;
    };

    var tasks = new List<Task<int>>();

    for (int i = 1; i <= 5; i++)
    {
        task = Task.Run(() => getPage(i));  
        tasks.Add(task);
    }

    Task.WaitAll(tasks.ToArray());
    //...etc
}

public static void SimulateSlowHttpRequest()
{
    var x = 1000000000;
    var multiplier = new Random().Next(8, 16);
    var y = x * multiplier;

    for (int i = 0; i <= y; i++)
    {
        var result = i++;
    }
}

I get the following output:

..or sometimes I might see a (e.g.) 3, but mostly 6s. If I change this line:

        task = Task.Run(() => getPage(i));

to:

        var j = i;
        task = Task.Run(() => getPage(j));

then I get the expected output (numbers 1 to 5 on each line, albeit in a random order).

I suspect this is because: a) the Tasks in the loop might start running at some undefined time after the for loop has finished, and b) i is being passed by reference to the Func, and the last value of it was 6

My question is why does it work with the above change above (i.e. making a local copy of i) and what is a better design for the above? The above code is basically the same structure I am working with for a program which makes concurrent calls to a REST service to get data that is to be returned in one result.

It's not b, for sure. nothing gets passed by reference without the use of the `ref` keyword, not even reference types. Also, the last value of i is 5. — Zohar Peled, Sep 19 '17 at 07:47
See also : https://blogs.msdn.microsoft.com/ericlippert/2009/11/12/closing-over-the-loop-variable-considered-harmful/ - this is a well documented issue regarding deferred (or lazy) execution when using lambda's. — PaulF, Sep 19 '17 at 07:48
You should pass the index to the action: `Task.Factory.StartNew((state) => getPage((int)state), i)`. Because the task is started later on, your directly referenced 'i' is already incremented. — KBO, Sep 19 '17 at 08:00
Consider using a task factory: `Func> createTask = (number) => Task.Run(() => getPage(number));` outside for the for-loop. then you can do this: `for (int i = 1; i <= 5; i++) tasks.Add(createTask(i));`. — Michael Coxon, Sep 19 '17 at 08:01
"Lambdas close over variables, _not values_" - when you access a variable from a lambda, that variable gets moved into a class by the compiler. A single instance of that class exists for each instance of the owning type (for fields) or method execution (for locals). — Gusdor, Sep 19 '17 at 08:15

Task.Run lambda is taking wrong local variable from loop

0 Answers0