Why an iterator (.Net) would be unreliable in this code

Question

I have an example that I can break every time I use an iterator, but it works fine with a for loop. All code uses variables local to the execution method. I'm at a dead end. There is either a fact about iterators that I don’t know about, or there is an honesty error in .Net. I bet on the first. Please help.

This code works reliably every time. It goes through (let’s say 10) all the elements one by one and starts a new thread, passing an integer to the new thread as an argument in the method. It starts 10 threads, one for each element. 1,2,3,4,5,6,7,8,9,10 - this always works.

WORKING CODE:

//lstDMSID is a populated List<int> with 10 elements. for(int i=0; i<lstDMSID.Count; i++) { int dmsId = lstDMSID[i]; ThreadStart ts = delegate { // Perform some isolated work with the integer DoThreadWork(dmsId); }; Thread thr = new Thread(ts); thr.Name = dmsId.ToString(); thr.Start(); } 

And this code actually repeats the elements. It iterates through (let’s say 10) all the elements one at a time and starts a new thread. It starts 10 threads, but does not reliably receive all 10 integers. I see that it starts 1,2,3,3,6,7,7,8,9,10. I am losing numbers.

BUSTED CODE:

 //lstDMSID is a populated List<int> with 10 elements. foreach(int dmsId in lstDMSID) { ThreadStart ts = delegate { // Perform some isolated work with the integer DoThreadWork(dmsId); }; Thread thr = new Thread(ts); thr.Name = dmsId.ToString(); thr.Start(); } 

Answer 1

The problem is based on the closure generated for your scope ...

The same problem would occur in your for loop if you rewrite it like this ( BAD CODE! ):

 // ...Closure now happens at this scope... for(int i=0;i<lstDMSID.Count;i++) { ThreadStart ts = delegate { DoThreadWork(lstDMSID[i]); // Eliminate the temporary, and it breaks! }; Thread thr = new Thread(ts); thr.Name = dmsId.ToString(); thr.Start(); } 

The problem is that when you close a variable in a delegate (in your case dmsId ), it closes in the area in which the variable is declared. When you use a for or foreach loop, closure occurs in the scope of the for / foreach statement, which is one level too high.

By introducing a temporary variable inside , the foreach loop will fix the problem:

 foreach(int dmsId in lstDMSID) { int temp = dmsId; // Add temporary ThreadStart ts = delegate { DoThreadWork(temp); // close over temporary, and it fixed }; Thread thr = new Thread(ts); thr.Name = dmsId.ToString(); thr.Start(); } 

For a more detailed discussion of what is happening, I would recommend reading Eric Lippert's blog post: "Closing a loop variable is considered harmful . "

Answer 2

This is because of the scope of the variable that you are using in closure.

Eric Lippert has a good blog post explaining this in detail, and I think others (John Skeet?) Wrote about this, too.

Answer 3

When executing an anonymous method in foreach , the compiler basically generates a class that points to dmsId. Thus, as the threads begin, each one points to the same variable, so depending on when the threads are scheduled, you will see that the numbers are duplicated or skipped.

In a for loop, you create a copy of an integer, so each thread gets its value.

There is some good data on this issue here .

Answer 4

The problem is that closures are closed over variables, not values. This means that all delegates receive a reference to the same variable, and the value of the variable changes each time through a loop

This should fix:

 //lstDMSID is a populated List with 10 elements. foreach(int dmsId in lstDMSID) { int tempId = dmsId; ThreadStart ts = delegate { //this is method that goes off ad does some isolated work with the integer DoThreadWork(tempId); }; Thread thr = new Thread(ts); thr.Name = tempId.ToString(); thr.Start(); } 

Answer 5

In the first case, dmsId is declared within the for loop, each delegate captures its own "instance" of this variable.

In the second version, dmsId is declared for the entire scope of the foreach loop. Each delegate captures the same variable - this means that you are accessing the same variable from multiple threads without blocking - bad stuff can happen.