The Foreach Identifier and Closures

The foreach identifier and closures

Edit: this all changes in C# 5, with a change to where the variable is defined (in the eyes of the compiler). From C# 5 onwards, they are the same.

---

Before C#5

The second is safe; the first isn't.

With foreach, the variable is declared outside the loop - i.e.

Foo f;
while(iterator.MoveNext())
{
     f = iterator.Current;
    // do something with f
}

This means that there is only 1 f in terms of the closure scope, and the threads might very likely get confused - calling the method multiple times on some instances and not at all on others. You can fix this with a second variable declaration inside the loop:

foreach(Foo f in ...) {
    Foo tmp = f;
    // do something with tmp
}

This then has a separate tmp in each closure scope, so there is no risk of this issue.

Here's a simple proof of the problem:

    static void Main()
    {
        int[] data = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
        foreach (int i in data)
        {
            new Thread(() => Console.WriteLine(i)).Start();
        }
        Console.ReadLine();
    }

Outputs (at random):

1
3
4
4
5
7
7
8
9
9

Add a temp variable and it works:

        foreach (int i in data)
        {
            int j = i;
            new Thread(() => Console.WriteLine(j)).Start();
        }

(each number once, but of course the order isn't guaranteed)

Closures behaving differently in for and foreach loops

In C# 5 and beyond, the foreach loop declares a separate i variable for each iteration of the loop. So each closure captures a separate variable, and you see the expected results.

In the for loop, you only have a single i variable, which is captured by all the closures, and modified as the loop progresses - so by the time you call the delegates, you see the final value of that single variable.

In C# 2, 3 and 4, the foreach loop behaved that way as well, which was basically never the desired behaviour, so it was fixed in C# 5.

You can achieve the same effect in the for loop if you introduce a new variable within the scope of the loop body:

for (int i = 3; i <= 4; i++)
{
    int copy = i;
    actions.Add(() => Console.WriteLine(copy));
}

For more details, read Eric Lippert's blog posts, "Closing over the loop variable considered harmful" - part 1, part 2.

Is LINQifying my code worth accessing a foreach variable in a closure?

You have two different issues here, one LINQ vs foreach, and the other is a different case.

Regarding the ReSharper informing you of "Access to foreach variable in closure..." when the code is LINQified - I just never take my chances, and leave it as a foreach loop. In most cases it is also more readable and maintainable, and really, shortening the code isn't that much of a big deal.

Regarding the second case - you'll need to lose the using statement, since the db object will be disposed too soon. You should close and dispose it in the "old school fashion" INSIDE the RunInTransaction lambda expression, at the end of it.

C# thread safe closure inside foreach

If you're on C# 5, then your code is fine; the semantics of foreach have been changed such that the loop variable is logically scoped to each iteration. Per Eric Lippert:

In C# 5, the loop variable of a foreach will be logically inside the loop, and therefore closures will close over a fresh copy of the variable each time.

If you're on C# 4 or earlier, then you should copy kvp to a local variable for the closure.

Your code has an unrelated bug: A task initialized through the Task constructor will not wait for the completion of the asynchronous function delegate that is passed to it as argument. Instead, you should use Task.Run for such delegates.

There is a simpler (safe) way of achieving this:

var tasks = result.Select(kvp => Task.Run(async () =>
{
    int retries = 0;
    bool success = false;
    // ...
})).ToList();

await Task.WhenAll(tasks);

Type and identifier are both required in a foreach statement using an object

With foreach, you need to setup the type for the parameter, as so.

foreach(dtIntegration_v10_r1.Vendor objvendor in objVendors)
{
    //your code.
}

---

Related Topics