Anonymous methods, captured variables and the Thread Pool

A

Anthony Paul

Hello everyone,

I just ran into a problem today that I found rather weird but can't
think of a reason why it's occurring, so I'm putting it out here to
see if any gurus have a better understanding of what's going on.

I have a method that I use to facilitate asynchronous operations such
that it accepts two delegate parameters, one that gets executed by a
background thread via the ThreadPool and the other a delegate to call
on the UI thread once the background thread is done. Works great, no
problems, but then I ran into a situation where I wanted to execute
multiple work delegates instead of just one, and once they are all
done only then would the UI delegate get called. So I did that, and
here's my modified method :

protected void DoAsyncStuff(ThreadStart[] workDelegates, ThreadStart
uiDelegate)
{
int workers = workDelegates.Length;

foreach (ThreadStart workDelegate in workDelegates)
{
ThreadPool.QueueUserWorkItem
(
(WaitCallback)delegate
{
workDelegate();
if (uiDelegate != null)
{
if
(Interlocked.Decrement(ref workers) == 0)
{

Dispatcher.BeginInvoke
(

(ThreadStart) delegate
{

uiDelegate();
},

System.Windows.Threading.DispatcherPriority.Normal
);
}
}
},
null
);
}
}

Now here's my method that uses it :

private void MyMethod()
{
int result1;
int result2;
DoAsyncStuff
(
new Delegate[]
{
delegate
{
result1 =
Model.GetRandomNumber();
},
delegate
{
result2 =
Model.GetRandomNumber();
}
},
delegate
{
// do something here with the
results
}
);
}

Everything executes fine, but when both work delegates have finished
executing and the UI delegate executes, result2 is the only one that
has a value. result1 didn't get updated. If I reverse the order,
again, the first delegate doesn't seem to return a value, but the
second one does. Does anyone understand what's going on here?

Regards,

Anthony
 
A

Anthony Paul

Ahhh yes of course! I'm such an idiot, I know all about the scope of
captured variables (as you can see in the method calling DoAsyncStuff)
but *completely* missed the boat in the DoAsyncStuff method. Thank
you, Pete, for pointing out what should have been obvious to me!
Doh! :D

[...]
Everything executes fine, but when both work delegates have finished
executing and the UI delegate executes, result2 is the only one that
has a value. result1 didn't get updated. If I reverse the order,
again, the first delegate doesn't seem to return a value, but the
second one does. Does anyone understand what's going on here?

Yes.  You've made the classic error in dealing with captured variables: 
reusing a loop variable for each delegate instance, rather than providing 
a new copy of the loop variable for each.

By the time your threads get to run, the loop has finished, and the loop  
variable is equal to the last value it had.  Thus, both threads use the 
same value.  In this case, the value is a delegate itself, and so that  
last delegate is the only one to execute, and it executes twice (in your  
example, because there are two elements in the array).

The solution is to copy the loop variable to a local variable inside the  
loop block, so that each delegate instance gets its own copy:

     foreach (ThreadStart workDelegate in workDelegates)
     {
         ThreadStart workCaptured = workDelegate;

         ThreadPool.QueueUserWorkItem(
             delegate()
             {
                 workCaptured();
                 if (uiDelegate != null)
                 {
                     // other stuff
                 }
             }, null);
     }

And please, next time ease off the tab-stops.  The indentation in your  
posted code was a bit ridiculous.  :)

Pete
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top