QueueUserWorkItem with delegate does not work, but WaitCallBack does work

Question

In the question below, I found this neat trick for calling QueueUserWorkItem in a type safe way, where you pass a delegate instead of WaitCallBack and an object. However, it doesn't work the way one would expect.

What's the difference between QueueUserWorkItem() and BeginInvoke(), for performing an asynchronous activity with no return types needed

Here's some sample code and output that demonstrates the issue.

for (int i = 0; i < 10; ++i)
{
    // doesn't work - somehow DoWork is invoked with i=10 each time!!!
    ThreadPool.QueueUserWorkItem(delegate { DoWork("closure", i); });

    // not type safe, but it works
    ThreadPool.QueueUserWorkItem(new WaitCallback(DoWork), Tuple.Create("    WCB", i));
}

void DoWork(string s, int i)
{
    Console.WriteLine("{0} - i:{1}", s, i);
}

void DoWork(object state)
{
    var t = (Tuple<string, int>)state;
    DoWork(t.Item1, t.Item2);
}

and here is the output:

closure - i:10
    WCB - i:0
closure - i:10
    WCB - i:2
    WCB - i:3
closure - i:10
    WCB - i:4
closure - i:10
    WCB - i:5
closure - i:10
    WCB - i:6
closure - i:10
    WCB - i:7
closure - i:10
    WCB - i:8
closure - i:10
    WCB - i:9
    WCB - i:1
closure - i:10

Note that when using the closure to call QueueUserWorkitem, i=10 for ever call, but when using the WaitCallBack you get the correct values, 0-9.

So my questions are:

1. Why isn't the correct value of i passed when using the closure/delegate way of doing it?
2. How on earth does i ever get to be 10? In the loop, it only ever had values 0-9 right?

Answer 1

The answers to both of your question are related to the scope of the closure when you create the anonymous method.

When you do this:

// Closure for anonymous function call begins here.
for (int i = 0; i < 10; ++i)
{
    // i is captured
    ThreadPool.QueueUserWorkItem(delegate { DoWork("closure", i); });
}

You're capturing i across the entire loop. That means that you queue up your ten threads very quickly, and by the time they start, the closure has captured i to be 10.

To get around this, you reduce the scope of the closure, by introducing a variable inside the loop, like so:

for (int i = 0; i < 10; ++i)
{
    // Closure extends to here.
    var copy = i;

    // **copy** is captured
    ThreadPool.QueueUserWorkItem(delegate { DoWork("closure", copy); });
}

Here, the closure doesn't extend beyond the loop, but just to the value inside.

That said, the second call to the QueueUserWorkItem produces the desired result because you've created the Tuple<T1, T2> at the time that the delegate is being queued up, the value is fixed at that point.

Note that in C# 5.0, the behavior for foreach was changed because it happens so often (where the closure closes over the loop) and causes a number of people a lot of headaches (but not for like you are using).

If you want to take advantage of that fact, you can call the Range method on the Enumerable class to use foreach:

foreach (int i in Enumerable.Range(0, 10))
{
    // Closure for anonymous function call begins here.
    ThreadPool.QueueUserWorkItem(delegate { DoWork("closure", i); });
}

Answer 2

That's because of how variables are captured: the delegate will take the value of i at the time of actual execution, not at the time of declaration, so by that time they're all 10. Try a copy to a local variable:

for (int i = 0; i < 10; ++i)
{
    int j = i;        
    ThreadPool.QueueUserWorkItem(delegate { DoWork("closure", j); });