ThreadPool, ManualResetEvent and WaitOne()

[rbDeveloper]

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]]

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

 

[rbDeveloper]

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]]

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

Right - as expected.

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.

 

[rbDeveloper]

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]]

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...