2

I am not really comfortable with multi threaded programming and while I was trying to implement it in my code, was running into an exception which I am not able to figure out why. Any help in this would be greatly appreciated :)
So, basically I have this small snippet of code:

string[][] Array1 = new string[thread_count][];

/* Logic to insert data in Array1 */

Thread[] WorkerThreads = new Thread[thread_count];

for (int i = 0; i < thread_count; i++)
{
    /* THE EXCEPTION OCCURS IN THE FOLLOWING LINE */
    WorkerThreads[i] = new Thread(() => GetVal(Array1[i], val, num));
    WorkerThreads[i].Start();
}

for (int i = 0; i < WorkerThreads.Length; i++)
    WorkerThreads[i].Join();  

Now, the value for thread_count is set to 10 and I am getting an IndexOutOfRange exception. The debugger shows the value of i as 10 and Array1[10][] is the one it is trying to access.
I am not getting how the value of i can reach 10 when the loop is not supposed to run that far.
Can anyone point out where am I going wrong? I am using C#.

Thanks

sachin11
  • 1,087
  • 3
  • 13
  • 17

5 Answers5

3

You have the "closing over loop variable" problem, check this: closing over the loop variable considered harmful and The foreach identifier and closures.

Add a temp variable to fix it:

for (int i = 0; i < thread_count; i++)
{
    var j = i;
    /* THE EXCEPTION OCCURS IN THE FOLLOWING LINE */
    WorkerThreads[j] = new Thread(() => GetVal(Array1[j], val, num));
    WorkerThreads[j].Start();
}
Community
  • 1
  • 1
fcuesta
  • 4,429
  • 1
  • 18
  • 13
  • I follow that the closures are all referencing the same variable `i` -- rather than the __value__ `i` had in that loop. But why does creating a temp variable `j` fundamentally change how the binding/ scope? – Thomas W Aug 06 '13 at 03:52
  • 1
    Because "j" has local scope inside the loop, and therefore a new "j" is created for each iteration of the loop. Each lambda expression gets its own copy. While "i" is in the outer scope to the loop, so the variable is reused in each iteration, and each lambda expression gets the same variable, reused also after the loop. – fcuesta Aug 06 '13 at 04:02
  • Thanks @fcuesta -- I can analyze that logically, but I'm not sure I get it intuitively yet :) Same situation comes up in Javascript I believe, especially with jQuery "handler" functions. – Thomas W Aug 07 '13 at 02:18
2

Your problem is the lambda/anonymous function you are creating. The variable i gets a value of 10 right before the loop exits. When you call new Thread(() => GetVal(Array1[i], val, num));, you have not actually called the code in question. The anonymous function keeps a reference to the variable i, and when you attempt to start the thread, it looks up the reference, sees it has a value of 10, and you get your exception.

Refer to the "Variable Scope in Lambda Expressions" section of this webpage: http://msdn.microsoft.com/en-us/library/vstudio/bb397687.aspx

nhellwig
  • 106
  • 3
1

You are likely running into a race conditions, try using ConcurrentBag or BlockingCollection from the System.Collections.Concurrent namespace, they make threaded programming easier.

ryudice
  • 36,476
  • 32
  • 115
  • 163
1

try below

for (int i = 0; i < thread_count; i++)
{
    int copy = i;
    WorkerThreads[copy] = new Thread(() => GetVal(Array1[copy], val, num));
    WorkerThreads[copy].Start();
}
Damith
  • 62,401
  • 13
  • 102
  • 153
0

GetVal(Array[i]) is causing closure over the variable i, which i will be equal to thread_count before the threads are created. All threads will have GetVal(Array[10]) in this case, since they all reference the latest i variable.

To resolve this, create a variable assigned to Array[i] before using it in GetVal:

for (int i = 0; i < thread_count; i++)
{
    string[] value = Array1[i];

    WorkerThreads[i] = new Thread(() => GetVal(value, val, num));
    WorkerThreads[i].Start();
}
matth
  • 6,112
  • 4
  • 37
  • 43