2
class Thread1_8
{
    static int shared_total;
    static int thread_count;
    static readonly object locker = new object();

    static void Main()
    {
        int[,] arr = new int[5, 5] { { 1,2,3,4,5}, {5,6,7,8,9}, {9,10,11,12,13}, {13,14,15,16,17}, { 17,18,19,20,21} };

        for(int i = 0; i < 5; i++)
        {
            new Thread(() => CalcArray(i, arr)).Start();
        }


        while(thread_count < 5)
        {

        }
        Console.WriteLine(shared_total);
    }

    static void CalcArray(int row, int[,] arr)
    {

        int length = arr.GetLength(0);
        int total = 0;
        for(int j = 0; j < length; j++)
        {
           total += arr[row,j];
        }

        lock (locker)
        {
            shared_total += total;
            thread_count++;
        }
    }
}

I keep getting a System.IndexOutOfRangeException.

The reason is because for some reason my "i" in the initial for loop is being set to 5 and STILL entering the loop for some reason. I don't understand why. At first I thought it might be because the each thread I create is incrementing the Array but it shouldn't since a Thread has a complete separate execution path, it should jump straight to the CalcArray method.

Luke Xu
  • 2,302
  • 3
  • 19
  • 43
  • 2
    [Don't close over a for loop variable.](http://stackoverflow.com/questions/227820/why-is-it-bad-to-use-an-iteration-variable-in-a-lambda-expression) Not a duplicate question per se, but this sort of question is asked in many different forms. – Mike Zboray Jan 15 '16 at 00:23
  • No but wait a minute, I never increment "i" or even touch it. So regardless it shouldn't matter? – Luke Xu Jan 15 '16 at 00:23
  • Also if you are going to lock access to a field you need to use the lock statement everywhere where it is accessed. – Mike Zboray Jan 15 '16 at 00:24
  • Could you explain where else I need to place the lock? I think it's not necessary to call lock on the Console.Write since there are no threads there? – Luke Xu Jan 15 '16 at 00:25
  • Yes you do increment `i`. In the iterator of the for loop. – Mike Zboray Jan 15 '16 at 00:25
  • To lock properly, would have to use something like `while(true) { lock (locker) { if (thread_count >= 5) { break; } } }`. Better yet don't use a busy wait (that can peg the CPU), and hold to the Thread objects and call Join on each one. Better still, use Tasks with `Task.Run` and `Task.WaitAll`. – Mike Zboray Jan 15 '16 at 00:31
  • I was trying to do that but how do you hold all the thread objects? I tried to use an array but it wouldn't let me declare the array without the delegate.So I'd have to pass CalcArray(i, arr) but I can't declare the Thread Array in the loop obviously. (So I don't have access to "i") – Luke Xu Jan 15 '16 at 00:33

1 Answers1

6

The reason why this happens is subtle: when you do this

for(int i = 0; i < 5; i++) {
    new Thread(() => CalcArray(i, arr)).Start();
}

variable i goes through values 0 all the way to 5, at which point the loop stops, because 5 < 5 evaluates to false.

The thread starts after the loop is over, so the value of row that it sees is 5, not 0 through 4.

This can be fixed by making a local variable row inside the loop.

Another problem in your code is checking thread_count inside Main without locking, in a busy loop. This is not the best process of synchronizing with your threads. Consider using ManualResetEventSlim instead.

Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • Thank you.. I think I see my flaw. I was just reading about this the other day but could you confirm this is the "captured variables" problem? – Luke Xu Jan 15 '16 at 00:27
  • Also, I just changed up my code to your solution and it worked. Thanks, will accept when it lets me. – Luke Xu Jan 15 '16 at 00:28
  • Really 6? Don't you mean 5? – 500 - Internal Server Error Jan 15 '16 at 00:28
  • I think the issue might also be fixable by replacing the `for` loop with `foreach(int i in Enumerable.Range(0,5))` . However, this only applies to C#5 and above; earlier versions [share variables between loop iterations](https://blogs.msdn.microsoft.com/ericlippert/2009/11/12/closing-over-the-loop-variable-considered-harmful/) for both `for` and `foreach` . – Brian Jan 15 '16 at 14:34