3
public void GatherDataFromSwitches(Device[] switches)
{
    List<Thread> workerThreads = new List<Thread>();
    for(int i = 0; i < switches.Length - 1; i++)
    {
        Thread t = new Thread(unused => GatherDataFromSwitch(switches[i]));
        workerThreads.Add(t);
        t.Start();
    }
    foreach (Thread d in workerThreads) d.Join(); //wait for all threads to finish
}

If I loop through switches after having run that method I notice that somehow some switches had no added data, and some switches had data added from multiple switches. So something went wrong with passing the reference to the worker threads. I'm still not sure what exactly, but I solved the problem by adding

Thread.Sleep(100); 

right after

t.Start();

I'm assuming this works because now the newly created thread has some time to initialize before the next is created. But this is a work around, not a fix. Is it because of how lambda expressions work?

How do I properly go around this?

Tudor
  • 61,523
  • 12
  • 102
  • 142

2 Answers2

3

The problem is the way i is captured in the lambda. Make a local copy inside the loop to have each lambda capture a distinct value:

public void GatherDataFromSwitches(Device[] switches)
{      
    List<Thread> workerThreads = new List<Thread>();
    for(int i = 0; i < switches.Length ; i++)
    {
        int j = i; // local i
        Thread t = new Thread(unused => GatherDataFromSwitch(switches[j]));
        workerThreads.Add(t);
        t.Start();
    }
    foreach (Thread d in workerThreads) d.Join(); //wait for all threads to finish
}

Or pass i explicitly as a parameter to the thread:

public void GatherDataFromSwitches(Device[] switches)
{      
    List<Thread> workerThreads = new List<Thread>();
    for(int i = 0; i < switches.Length ; i++)
    {
        Thread t = new Thread(param => { j = (int)param; GatherDataFromSwitch(switches[j]); });
        workerThreads.Add(t);
        t.Start(i);
    }
    foreach (Thread d in workerThreads) d.Join(); //wait for all threads to finish
}
Tudor
  • 61,523
  • 12
  • 102
  • 142
  • This indeed fixes it, I orginally started with a foreach loop. It seems odd having to copy the object in order to pass it to lambda. – Maximiliaan Aelvoet Jun 07 '12 at 09:44
  • @Maximiliaan Aelvoet: In fact, I much prefer the second version, which makes it more clear that you are passing the index as a parameter to the thread. At least for me it's more clear, since I started programming with pthreads. :) Btw, don't forget to mark the answer as accepted in case it solved your problem. – Tudor Jun 07 '12 at 09:50
  • Just to explain a bit: the problem with the initial version was the fact that the lambda takes the value `i` at the time when it's actually executed. Since the threads you start run asynchronously, there is no way to know what value `i` will have by the time each thread reaches the code that uses `i`. – Tudor Jun 07 '12 at 09:53
  • So if I get this right, the value `i` only gets read by lambda when I call the `t.Start()` but because right after that the next iteration starts, the `i` value is changed and thus I get the weird results? If so, thanks for the explanation! – Maximiliaan Aelvoet Jun 07 '12 at 10:02
  • But the lambda only starts after the thread has been started? – Maximiliaan Aelvoet Jun 07 '12 at 10:06
  • Yes, you are correct. The value of `i` seen by the lambda is the value of when the lambda starts executing (when the thread gets to run), not the value of `i` when the lambda is created. – Tudor Jun 07 '12 at 10:34
0
public void GatherDataFromSwitches(Device[] switches) {
     List<Thread> workerThreads = new List<Thread>();
     for(int i = 0; i < switches.Length ; i++)
     {
         int j = i;
         Thread t = new Thread(unused => GatherDataFromSwitch(switches[j]));
         workerThreads.Add(t);
         t.Start();
     }
     foreach (Thread d in workerThreads) d.Join(); //wait for all threads to finish 
}

note the copying of i to j. If you had resharper, your code will produce warning, more on this in question Access to Modified Closure

Community
  • 1
  • 1
nothrow
  • 15,882
  • 9
  • 57
  • 104