V3129. The value of the captured variable will be overwritten on the next iteration of the loop in each instance of anonymous function that captures it.


The analyzer detected a potential error related to anonymous function closure of a variable which is used as a loop iterator. At the compile time, the captured variable will be wrapped in a container class, and from this point on only one instance of this class will be passed to all anonymous functions for each iteration of a loop. Most likely, the programmer will expect different values of an iterator inside every anonymous function instead of the last value, which is an unobvious behavior and may cause an error.

Let's take a closer look at this situation using the following example:

void Foo()
{
  var actions = new List<Action>();
  for (int i = 0; i < 10; i++)
  {
    actions.Add(() => Console.Write(i)); // <=
  }
  // SOME ACTION
  actions.ForEach(x => x());
}

It is popularly believed that after executing the 'Foo' method, the numbers from 0 to 9 will be displayed on a console, because logically after the 'i' variable is enclosed in anonymous function, after compiling an anonymous container class will be created, and a value of the 'i' variable will be copied into one of its fields. But actually, the number 10 will be printed on console 10 times. This is caused by the fact that an anonymous container class will be created immediately after the 'i' variable declaration and not before the anonymous function declaration. As a result, all instances of the anonymous function on each loop enclose not the current value of the iterator, but a reference to the anonymous container class, which contains the last value of the iterator. It is also important to note that during the compilation, the 'i' variable declaration will be placed before the loop.

Picture 1

Fig.1. Comparison of C# and IL code

To avoid this error, ensure that the anonymous function encloses a local variable for the current iteration. The corrected code would look like:

void Foo()
{
  var actions = new List<Action>();
  for (int i = 0; i < 10; i++)
  {
    var curIndex = i;
    actions.Add(() => Console.Write(curIndex)); // <=
  }
  // SOME ACTION
  actions.ForEach(x => x());
}

Therefore, we copy the value of the iterator to the local variable at each iteration and, as we already know, the anonymous container class will be created during the declaration of the enclosed variable, in our case - during the declaration of the 'curIndex' variable containing the current iterator value.

Let's examine a suspicious code fragment from the 'CodeContracts' project:

var tasks = new Task<int>[assemblies.Length];
Console.WriteLine("We start the analyses");
for (var i = 0; i < tasks.Length; i++)
{
    tasks[i] = new Task<int>(() => CallClousotEXE(i, args)); // <=
    tasks[i].Start();
}
Console.WriteLine("We wait");
Task.WaitAll(tasks);

Despite the fact that the task (Task) is created and started during the same iteration, it will not be started immediately. Therefore, the chances are high that the task will start after the current iteration which will cause an error.

For example, if we run the given piece of code in a synthetic test we will see that all tasks were started after the loop is complete, and, thereby, the 'i' variable in all tasks is equal to the last iterator value (10).

The corrected code would look like:

var tasks = new Task<int>[10];
Console.WriteLine("We start the analyses");
for (var i = 0; i < tasks.Length; i++)
{
    var index = i;
    tasks[i] = new Task<int>(() => CallClousotEXE(index, args));
    tasks[i].Start();
}
Console.WriteLine("We wait");
Task.WaitAll(tasks);

Bugs Found

Checked Projects
336
Collected Errors
12 743