Anonymous methods, captured variables and the Thread Pool

  • Thread starter Thread starter Anthony Paul
  • Start date Start date
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
 
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?
Click to expand...

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
Click to expand...
 
Back
Top