5

Possible Duplicate:
C# Captured Variable In Loop

I'm working on a few simple applications of threading, but I can't seem to get this to work:

class ThreadTest
{
    static Queue<Thread> threadQueue = new Queue<Thread>();

    static void Main()
    {
        //Create and enqueue threads
        for (int x = 0; x < 2; x++)
        {
            threadQueue.Enqueue(new Thread(() => WriteNumber(x)));
        }

        while(threadQueue.Count != 0)
        {
            Thread temp = threadQueue.Dequeue();
            temp.Start();
        }

        Console.Read();
    }

    static void WriteNumber(int number)
    {
        for (int i = 0; i < 1000; i++)
        {
            Console.Write(number);
        }
    }
}

The goal basically is to add threads to a queue one by one, and then to go through the queue one by one and pop off a thread and execute it. Because I have "x<2" in my for loop, it should only make two threads - one where it runs WriteNumber(0), and one where it runs WriteNumber(1), which means that I should end up with 1000 0's and 1000 1's on my screen in varying order depending on how the threads are ultimately executed.

What I end up with is 2000 2's. The two possible solutions that I've come up with are: I've missed something glaringly obvious, or sending the variable x to the WriteNumber function is doing a pass-by-reference and not pass-by-value, so that when the threads execute they're using the most recent version of x instead of what it was at the time that the function was set. However, it was my understanding that variables are passed by value by default in C#, and only passed by reference if you include 'ref' in your parameter.

Community
  • 1
  • 1
Jake
  • 3,142
  • 4
  • 30
  • 48
  • 2
    A queue of threads is certainly an unusual idea. – CodesInChaos Jun 19 '12 at 21:34
  • it s not an unsuaul idea that s what threadpool does internally. so use threadpool instad. – DarthVader Jun 19 '12 at 21:36
  • Threadpools usually have a queue of task class instances and, if they can be bothered to keep track, a fixed list of threads that dequeue and execute the tasks. Micromanaging threads in lists/queues usually leads to disaster. I've seen a few attempts at this on SO - big gobs of code trying to poll if threads in lists have 'finished', free them and ceate new ones to fill in the hole in the array. Truly horrifying... – Martin James Jun 20 '12 at 03:11

2 Answers2

18

You're capturing x in the lambda expression. The value of x changes to 2 before you start the threads. You need to make a copy of value within the loop:

for (int x = 0; x < 2; x++)
{
    int copy = x;
    threadQueue.Enqueue(new Thread(() => WriteNumber(copy)));
}

Lambda expressions capture variables. Even though the value passed to WriteNumber is passed by value, it's not called at all until the thread is started - by which time x is 2.

By making a copy within the loop, each iteration of the loop gets its own separate "instance" of the copy variable, and that doesn't change value... so by the time WriteNumber is called, each copy variable still has the same value that x had for that iteration.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Fascinating, thank you! Looks like I have a ton more research to do on lambda expressions! – Jake Jun 20 '12 at 00:18
9

This occurs because the value of x is accessed after the loop is finished, and by that time it is 2.

You need to use a temporary variable to prevent variable capturing.

for (int x = 0; x < 2; x++)
{
    int tmp = x;
    threadQueue.Enqueue(new Thread(() => WriteNumber(tmp)));
}
Kendall Frey
  • 43,130
  • 20
  • 110
  • 148