The foreach identifier and closures

Question

In the two following snippets, is the first one safe or must you do the second one?

By safe I mean is each thread guaranteed to call the method on the Foo from the same loop iteration in which the thread was created?

Or must you copy the reference to a new variable "local" to each iteration of the loop?

var threads = new List<Thread>(); foreach (Foo f in ListOfFoo) {           Thread thread = new Thread(() => f.DoSomething());     threads.Add(thread);     thread.Start(); } 

-

var threads = new List<Thread>(); foreach (Foo f in ListOfFoo) {           Foo f2 = f;     Thread thread = new Thread(() => f2.DoSomething());     threads.Add(thread);     thread.Start(); } 

Update: As pointed out in Jon Skeet's answer, this doesn't have anything specifically to do with threading.

Answer 1

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)