7

I am trying to run several tasks at the same time and I came across an issue I can't seem to be able to understand nor solve.

I used to have a function like this :

private void async DoThings(int index, bool b) {
    await SomeAsynchronousTasks();
    var item = items[index];
    item.DoSomeProcessing();
    if(b)
        AVolatileList[index] = item; //volatile or not, it does not work
    else
        AnotherVolatileList[index] = item;
}

That I wanted to call in a for loop using Task.Run(). However I could not find a way to send parameters to this Action<int, bool> and everyone recommends using lambdas in similar cases:

for(int index = 0; index < MAX; index++) { //let's say that MAX equals 400 
    bool b = CheckSomething();
    Task.Run(async () => {
        await SomeAsynchronousTasks();
        var item = items[index]; //here, index is always evaluated at 400
        item.DoSomeProcessing();
        if(b)
            AVolatileList[index] = item; //volatile or not, it does not work
        else
            AnotherVolatileList[index] = item;
    }
}

I thought using local variables in lambdas would "capture" their values but it looks like it does not; it will always take the value of index as if the value would be captured at the end of the for loop. The index variable is evaluated at 400 in the lambda at each iteration so of course I get an IndexOutOfRangeException 400 times (items.Count is actually MAX).

I am really not sure about what is happening here (though I am really curious about it) and I don't know how to do what I am trying to achieve either. Any hints are welcome!

Max
  • 3,453
  • 3
  • 32
  • 50

2 Answers2

7

Make a local copy of your index variable:

for(int index = 0; index < MAX; index++) {
  var localIndex = index;
  Task.Run(async () => {
    await SomeAsynchronousTasks();
    var item = items[index];
    item.DoSomeProcessing();
    if(b)
        AVolatileList[index] = item;
    else
        AnotherVolatileList[index] = item;
  }
}

This is due to the way C# does a for loop: there is only one index variable that is updated, and all your lambdas are capturing that same variable (with lambdas, variables are captured, not values).

As a side note, I recommend that you:

  1. Avoid async void. You can never know when an async void method completes, and they have difficult error handling semantics.
  2. await all of your asynchronous operations. I.e., don't ignore the task returned from Task.Run. Use Task.WhenAll or the like to await for them. This allows exceptions to propagate.

For example, here's one way to use WhenAll:

var tasks = Enumerable.Range(0, MAX).Select(index =>
  Task.Run(async () => {
    await SomeAsynchronousTasks();
    var item = items[localIndex];
    item.DoSomeProcessing();
    if(b)
        AVolatileList[localIndex] = item;
    else
        AnotherVolatileList[localIndex] = item;
  }));
await Task.WhenAll(tasks);
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
2

All your lambdas capture the same variable which is your loop variable. However, all your lambdas are executed only after the loop has finished. At that point in time, the loop variable has the maximum value, hence all your lambdas use it.

Stephen Cleary shows in his answer how to fix it.

Eric Lippert wrote a detailled two-part series about this.

Malcolm
  • 1,239
  • 1
  • 14
  • 25
Daniel Hilgarth
  • 171,043
  • 40
  • 335
  • 443