ThreadPool, ManualResetEvent and WaitOne()

[=?Utf-8?B?cmJEZXZlbG9wZXI=?=]

The following is from a simple Windows application in VS2005, which has
button1 and textbox1 dragged onto a form.

In StartThreads(), I call ThreadPool.QueueUserWorkItem(), then call
WaitOne(). My expectation is that I would see the text generated in
WasteTime() before seeing the "Hey" printout that comes after WaitOne().
Instead, I'm seeing the "Hey" as the first thing to print out in the text
box.

Any thoughts on why WaitOne() isn't waiting for the ThreadPool thread to
complete?

Thanks...


private void button1_Click(object sender, EventArgs e)
{
StartThreads();
}

private void StartThreads()
{
CalculationRequest cr = new CalculationRequest();
cr.UserID = "42";

ThreadPool.QueueUserWorkItem(new WaitCallback(WasteTime), cr);

cr.ProcessingEvent.WaitOne();

textBox1.Text += "Hey";
textBox1.Text += "\r\n";
}

private void WasteTime(object state)
{
if (state is CalculationRequest)
{
CalculationRequest cr = state as CalculationRequest;
cr.ProcessingEvent.Set();

for (int i = 0; i < 5; i++)
{
SetText(String.Format("threadId: {0}, count: {1}",
cr.UserID, i));
System.Threading.Thread.Sleep(100);
}

}
}

//The following allows us to set text in textbox1 from a
//ThreadPool thread.
delegate void SetTextCallback(string text);
private void SetText(string text)
{
if (this.textBox1.InvokeRequired)
{
SetTextCallback d = new SetTextCallback(SetText);
this.Invoke(d, new object[] { text });
}
else
{
this.textBox1.Text += String.Format("{0}\n", text);
this.textBox1.Text += "\r\n";
}
}
class CalculationRequest
{
public string UserID;

// Thead/Sync info
public ManualResetEvent ProcessingEvent = new
ManualResetEvent(false);
}

[Jon Skeet [C# MVP]]

On Jun 20, 5:41*pm, rbDeveloper
<[e-mail]:

The following is from a simple Windows application in VS2005, which has
button1 and textbox1 dragged onto a form.

In StartThreads(), I call ThreadPool.QueueUserWorkItem(), then call
WaitOne(). My expectation is that I would see the text generated in
WasteTime() before seeing the "Hey" printout that comes after WaitOne().

Why? WaitOne() will only wait until the ManualResetEvent is set, which
it is very early in the threadpool thread. If you move the call to Set
to the *end* of the threadpool operation, you'll see a difference.
However, I predict that it won't be the difference you want. I predict
it will hang.

Your main UI thread is blocking until the threadpool thread sets the
event - but the threadpool thread itself will block while the main
thread isn't processing the message loop, due to your use of
Control.Invoke. I suggest you change that Invoke call to BeginInvoke.
I also suggest you try a different way of handling the whole
situation, probably with a callback at the end of the threadpool
method. After all, if you're going to block the UI thread until
something else has finished, you might as well not be using a
threadpool thread in the first place.

Jon

[Peter Duniho]

On Fri, 20 Jun 2008 09:41:01 -0700, rbDeveloper
<rb*********@discussions.microsoft.comwrote:

[...]
Any thoughts on why WaitOne() isn't waiting for the ThreadPool thread to
complete?

Well, the immediate answer to that question is: because you are setting
the event before the thread completes.

It's hard to understand from the code you posted why you would expect it
to wait, given that the first thing you do in your thread is to set the
event.

That said, it's a very very bad idea to be waiting for the thread anyway.
Not only are you waiting in the GUI thread, which makes the GUI
unresponsive, but you've created a deadlock situation by blocking your GUI
thread at the same time that you're using Invoke() from another thread.
If you changed the code so that you didn't set the event before calling
Invoke(), then the GUI thread would be waiting for the worker thread while
the worker thread was waiting for the GUI thread.

Hopefully you see how that won't work. :)

Pete

[=?Utf-8?B?cmJEZXZlbG9wZXI=?=]

"Jon Skeet [C# MVP]" wrote:

Why? WaitOne() will only wait until the ManualResetEvent is set, which
it is very early in the threadpool thread. If you move the call to Set
to the *end* of the threadpool operation, you'll see a difference.
However, I predict that it won't be the difference you want. I predict
it will hang.

Hang is exactly what happens when moving set to the end with no other changes.

Your main UI thread is blocking until the threadpool thread sets the
event - but the threadpool thread itself will block while the main
thread isn't processing the message loop, due to your use of
Control.Invoke. I suggest you change that Invoke call to BeginInvoke.

I've changed to start the process on a backgroundworker thread, moved set to
the end, and changed Invoke to BeginInvoke. More importantly, I have a
clearer understanding of how using BeginInvoke and a ThreadPool works.

Many, many thanks for both Jon and Pete. Updated code below in case anyone
reading this wants to review.

Thanks again!
Randy


private void button1_Click(object sender, EventArgs e)
{
_BackgroundWorker.RunWorkerAsync();
}

private void _BackgroundWorker_DoWork(object sender, DoWorkEventArgs
e)
{
StartThreads();
}

private void StartThreads()
{
CalculationRequest cr = new CalculationRequest();
cr.UserID = "42";

ThreadPool.QueueUserWorkItem(new WaitCallback(WasteTime), cr);

cr.ProcessingEvent.WaitOne();

SetText("Hey");
SetText("\r\n");
}

private void WasteTime(object state)
{
if (state is CalculationRequest)
{
CalculationRequest cr = state as CalculationRequest;
for (int i = 0; i < 5; i++)
{
SetText(String.Format("threadId: {0}, count: {1}",
cr.UserID, i));
System.Threading.Thread.Sleep(100);
}
cr.ProcessingEvent.Set();
}
}

//The following allows us to set text in textbox1 from a
//ThreadPool thread.
delegate void SetTextCallback(string text);
private void SetText(string text)
{
if (this.textBox1.InvokeRequired)
{
SetTextCallback d = new SetTextCallback(SetText);
//this.Invoke(d, new object[] { text });
this.BeginInvoke(d, new object[] { text });
}
else
{
this.textBox1.Text += String.Format("{0}\n", text);
this.textBox1.Text += "\r\n";
}
}
class CalculationRequest
{
public string UserID;

// Thead/Sync info
public ManualResetEvent ProcessingEvent = new
ManualResetEvent(false);
}

[Jon Skeet [C# MVP]]

rbDeveloper <rb*********@discussions.microsoft.comwrote:

Why? WaitOne() will only wait until the ManualResetEvent is set, which
it is very early in the threadpool thread. If you move the call to Set
to the *end* of the threadpool operation, you'll see a difference.
However, I predict that it won't be the difference you want. I predict
it will hang.

Hang is exactly what happens when moving set to the end with no other changes.

Right - as expected.

Your main UI thread is blocking until the threadpool thread sets the
event - but the threadpool thread itself will block while the main
thread isn't processing the message loop, due to your use of
Control.Invoke. I suggest you change that Invoke call to BeginInvoke.

I've changed to start the process on a backgroundworker thread, moved set to
the end, and changed Invoke to BeginInvoke. More importantly, I have a
clearer understanding of how using BeginInvoke and a ThreadPool works.

However, you've still got a wait in the UI thread. While you're waiting
for the background threads to run, your UI will be useless. You won't
be able to resize it, if windows are moved over it the window won't be
repainted, etc. It's just not a nice thing to do.

--
Jon Skeet - <sk***@pobox.com>
Web site: http://www.pobox.com/~skeet
Blog: http://www.msmvps.com/jon.skeet
C# in Depth: http://csharpindepth.com

[=?Utf-8?B?cmJEZXZlbG9wZXI=?=]

Hi Jon,

Again, all your fantastic insights are greatly appreciated.

I believe control gets returned to the UI because the whole process is now
kicked off on a backgroundworker thread. Responsiveness returns immediately,
repainting is okay, etc. The only thing changed in the code below is that the
thread now runs for 7.5 seconds - long enough to roll other windows over.

Any insights you have area always appreciated.

Randy

private void button1_Click(object sender, EventArgs e)
{
_BackgroundWorker.RunWorkerAsync();
}

private void _BackgroundWorker_DoWork(object sender, DoWorkEventArgs
e)
{
StartThreads();
}

private void StartThreads()
{
CalculationRequest cr = new CalculationRequest();
cr.UserID = "42";

ThreadPool.QueueUserWorkItem(new WaitCallback(WasteTime), cr);

//WaitOne() until the ManualResetEvent is Set.
cr.ProcessingEvent.WaitOne();

SetText("Hey");
SetText("\r\n");
}

private void WasteTime(object state)
{
if (state is CalculationRequest)
{
CalculationRequest cr = state as CalculationRequest;

for (int i = 0; i < 15; i++)
{
SetText(String.Format("threadId: {0}, count: {1}",
cr.UserID, i));
System.Threading.Thread.Sleep(500);
}
//Set the ManualResetEvent();
cr.ProcessingEvent.Set();
}
}

//The following allows us to set text in textbox1 from a
//ThreadPool thread.
delegate void SetTextCallback(string text);
private void SetText(string text)
{
if (this.textBox1.InvokeRequired)
{
SetTextCallback d = new SetTextCallback(SetText);
//this.Invoke(d, new object[] { text });
this.BeginInvoke(d, new object[] { text });
}
else
{
this.textBox1.Text += String.Format("{0}\n", text);
this.textBox1.Text += "\r\n";
}
}
class CalculationRequest
{
public string UserID;

// Thead/Sync info
public ManualResetEvent ProcessingEvent = new
ManualResetEvent(false);
}

[Jon Skeet [C# MVP]]

rbDeveloper <rb*********@discussions.microsoft.comwrote:

Again, all your fantastic insights are greatly appreciated.

I believe control gets returned to the UI because the whole process is now
kicked off on a backgroundworker thread.

Ah, I do apologise - I'd missed that. Yes, that should be fine. Serves
me right for not reading closely enough.

However, I'm not sure I see the point of creating a BackgroundWorker
which then just hangs around waiting for something else. Can't you get
the threadpool thread to do the bit of work that needs to happen just
after it's completed the main job? If it makes the code a lot easier,
that's fine - but you may find it trickier to debug 3 threads than 2,
if it ever comes to that...

--
Jon Skeet - <sk***@pobox.com>
Web site: http://www.pobox.com/~skeet
Blog: http://www.msmvps.com/jon.skeet
C# in Depth: http://csharpindepth.com

[Peter Duniho]

On Fri, 20 Jun 2008 13:00:00 -0700, rbDeveloper
<rb*********@discussions.microsoft.comwrote:

Hi Jon,

Again, all your fantastic insights are greatly appreciated.

I believe control gets returned to the UI because the whole process is
now
kicked off on a backgroundworker thread. Responsiveness returns
immediately,
repainting is okay, etc. The only thing changed in the code below is
that the
thread now runs for 7.5 seconds - long enough to roll other windows over.

As Jon suggested, having a BackgroundWorker just sit there and wait for a
thread pool to finish seems sort of wasteful.

In fact, it seems to me that the best implementation would be to simply
have the BackgroundWorker itself do all of the work. That's what it's
for. Don't call "StartThreads()" in the DoWork event handler, just call
"WasteTime()". Then, subscribe a method to the BackgroundWorker's
RunWorkerCompleted event. Assuming you created the BackgroundWorker on
your GUI thread, the RunWorkerCompleted event handler itself will be
executed on your GUI thread, and you won't have to worry about using
Control.Invoke() or Control.BeginInvoke() at all.

As an example (I'm assuming you set up the RunWorkerCompleted event
handler as you do the DoWork handler):

private void button1_Click(object sender, EventArgs e)
{
_BackgroundWorker.RunWorkerAsync();
}

private void _BackgroundWorker_DoWork(object sender,
DoWorkEventArgs e)
{
CalculationRequest cr = new CalculationRequest();
cr.UserID = "42";

WasteTime(cr);
}

private void _BackgroundWorker_RunWorkerCompleted(object sender,
RunWorkerCompletedEventArgs e)
{
SetText("Hey");
SetText("\r\n");
}

private void WasteTime(CalculationRequest cr)
{
for (int i = 0; i < 15; i++)
{
SetText(String.Format("threadId: {0}, count: {1}",
cr.UserID, i));
System.Threading.Thread.Sleep(500);
}
}

private void SetText(string text)
{
this.textBox1.Text += String.Format("{0}\n", text);
this.textBox1.Text += "\r\n";
}

class CalculationRequest
{
public string UserID;
}

BackgroundWorker can pass an object to the DoWork event handler if you
want the button click code to do some of the initialization. Basically,
it offers all of the features of directly using the thread pool, but adds
the cross-thread marshalling for the appropriate events (it also has a
ProgressChanged event that is also raised on the GUI thread).

Hope that helps.

Pete