Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Using anonymous methods inside of a delayed task inside of a loop

I have the following -

for (int i = 0; i < N; ++i)
{
    var firstTask = DoSomething(...);

    var task = anotherTask.ContinueWith(
        (t) => ProcessResult(i, t.Result));
}

Problem being that the value of i passed into ProcessResult seems to be the value when it starts, not the value of the iteration when it is created.

What is the best way to protect against this?

like image 359
Hector Avatar asked Jan 12 '17 08:01

Hector


3 Answers

You need to capture the value of i into its own variable.

for (int i = 0; i < N; ++i)
{
    var count = i;

    var firstTask = DoSomething(...);

    var task = anotherTask.ContinueWith(
        (t) => ProcessResult(count, t.Result));
}

Example:

for (int i = 0; i < 5; ++i)
{
    var a = i;
    var task = Task.Delay(0).ContinueWith((t) => a.Dump());
}

This outputs something like:

0
2
1
4
3

But this:

for (int i = 0; i < 5; ++i)
{
    var task = Task.Delay(0).ContinueWith((t) => i.Dump());
}

Outputs:

5
5
5
5
5
like image 86
Hein Andre Grønnestad Avatar answered Oct 14 '22 10:10

Hein Andre Grønnestad


You need to create a temp variable inside the loop; in your current code you are capturing the variable i, not the value, which means that when the continuation tasks are finally executed the loop has already finished and i is N-1.

 for (int i = ...)
 {
     var temp = i;
     var task = anotherTask.ContinueWith(t => ProcessResult(temp, t.Resume));
 }
like image 2
InBetween Avatar answered Oct 14 '22 10:10

InBetween


A lambda that uses an outer variable actually captures the variable, not the value stored in it. That means that as looping proceeds, the value you would read from the captured variable also changes.

You can fix this by using a temp variable inside the loop. Your code would be a lot cleaner though if you used async/await instead of ContinueWith and lambdas, eg:

for (int i=0;i<N;i++)
{
    //...
    var result=await thatOtherAsyncMethod(...);
    ProcessResult(i, result));
}

In general, you can avoid capture problems by copying the loop variable into a variable defined inside the loop's scope. 

This fixes the problem because the temporary variable exists only inside the loop's body. The lambda is also created inside the loop's body and captures a local, unchanging variable:

for (int i=0;i<N;i++)
{
    var temp=i;
    var myLambda = new Action(()=>MyMethod(temp));

    //This runs with the local copy, not i
    myLambda();
}

An even better way though, is to avoid captures and pass the loop value as the state parameter to ContinueWith, eg:

for (int i = 0; i < N; ++i)
{
    //...
    var task = anotherTask.ContinueWith(
                               (t,state) => ProcessResult((int)state, t.Result),
                               i);
    //...
}
like image 2
Panagiotis Kanavos Avatar answered Oct 14 '22 09:10

Panagiotis Kanavos