CODEWITHSUNDEEP
c#multithreading
Darren Hoehna
Darren Hoehna

Reputation: 379

Thread alters passed Int, if start() is called separately

I've read some things on threading and know that I should lock up a variable that is being accessed by multiple threads. The problem that I am experiencing is that the value in MainQueueToWorkingQueue(x) is always three if I kick off the thread by first allocating the thread, then calling start in the second for loop.

But, if I do this new Thread(() => MainQueueToWorkingQueue(x)).Start(); then the code runs as expected and the correct number is passed.

        private static List<BlockingQueue> WorkingQueueList = new List<BlockingQueue>();
static void Main(string[] args)
{
        for(Int32 WorkingQueueListInsers = 0; WorkingQueueListInsert < 3; WorkinfQueueListInsert++)
        {
           WorkingQueueList.Add(new BlockingQueue(20));
        }
        Thread[] MainThreads = new Thread[WorkingQueueList.Count];

        for (Int32 x = 0; x < WorkingQueueList.Count; x++)
        {
            /* MainThreads[x] = */ new Thread(() => MainQueueToWorkingQueue(x)).Start();
            Thread.Sleep(50);
        }

        for (Int32 x = 0; x < WorkingQueueList.Count; x++)
        {
            MainThreads[x].Start();
            Thread.Sleep(50);
        }
        Console.Read();
    }


    private static void MainQueueToWorkingQueue(Int32 WorkingQueuePosition)
    {
        /* if new Thread(() => MainQueueToWorkingQueue(x)).Start(); is called
             then WorkingQueuePosition is correct and is either zero, one, or two. */

        /* if the thread is allocated in one for loop, then started in another for loop,  WorkingQueuePosition only equals three */
        Console.WriteLine("Ending x: " + WorkingQueuePosition);
        while (true)
        {
            WorkingQueueList[WorkingQueuePosition].Enqueue(MainQueue.Dequeue());
        }
    }

My question is this. Why is the passed parameter correct when I use Start() when making the new thread, but the passed parameter is always three when I call Start() in a second for loop?

My guess: I know somewhere the parameter is being altered. My first guess was that the loop was running to fast that the thread used a different value than the one passed because x was updated too quickly. I tried fixing that with a Thread.Sleep(50) but the issue still remained.

EDIT: Took out code that did not deal directly with the problem.

Upvotes: 0

Views: 70

Answers (1)

Iridium
Iridium

Reputation: 23731

Your problem is in the use of lambda expressions here where you are forming a closure over the loop variable x, not its value at that point. As a result, when the threads actually start, the value of x may have changed (because the loop has executed further iterations) and it is the new value that is passed to your methods.

If you are using ReSharper, it will warn you of an "Access to modified closure".

Note that your "working" version is still vulnerable the problem, but as the threads are started in the loop, there's a greater chance that the value of x won't have changed (especially with your 50ms sleep in there). However if one of the threads takes > 50ms to start up, you will still see the wrong value.

You can fix this by copying the value of x to a local variable inside the loop. This will fix the code in both cases - whether you start the threads in this loop, or store the threads into the MainThreads/WorkingThreads arrays and start them later. Your 50ms sleep should also no longer be required.

    for (Int32 x = 0; x < WorkingQueueList.Count; x++)
    {
        var localX = x;
        Console.WriteLine("Starting x: " + x);
        /* MainThreads[x] = */ new Thread(() => MainQueueToWorkingQueue(localX)).Start();
        /* WorkingThreads[x]  =*/ new Thread(() => WorkingQueueToJob(localX)).Start();
    }

You can read more about this issue here: http://blogs.msdn.com/b/ericlippert/archive/2009/11/12/closing-over-the-loop-variable-considered-harmful.aspx

Upvotes: 3

Related Questions