GUI and worker deadlocking due to lock and Control.Invoke...

[User N]

I have a log class with static methods that grab and release a mutex as
required. The log class is designed to be called from both GUI and
thread pool threads, and dump output to a logfile and a RichEditBox.
To give you some idea of the approach I took...

private static RichTextBox rtb;
private delegate void RtbAppendTextHandler(string output);
private static RtbAppendTextHandler RtbAppendText;

Log.Init(RichTextBox richTextBox, string logFile)
{
rtb = richTextBox;
RtbAppendText = new RtbAppendTextHandler(rtb.AppendText);
Log.CreateLogFile(logFile);
}

Log.Write(string msg)
{
mutex.WaitOne();
string output = DateTime.Now.ToString() + " : " + msg;
rtb.Invoke(RtbAppendText, new object[] {output});
if(logToFile)
Log.WriteLogFile(output);
mutex.ReleaseMutex();
}

What appears to be happening is a worker thread is grabbing the mutex
and before the rtb.Invoke call the GUI thread waits on the mutex and
there things lockup. Which I take makes some sense, even though it
isn't what I'd like to happen. What I'm unsure of is how to best resolve
this issue.

The only place I manipulate the RichTextBox is in Log methods which
first wait for the mutex. Based on some things I've read, I got the
impression that that alone isn't enough to assure thread safety and when
one wants to manipulate a control from a worker thread they must do so
via Control.[Begin]Invoke. However, for fun, I tried changing the
rtb.Invoke(...) to an rtb.AppendText(output) and hitting the Log with
threads that write at a pretty good clip. Everything worked OK. I
then commented out the lines that wait/release on the mutex and tried
again and quickly got an exception. This leaves me wondering if the
mutex does in fact provide adequate protection. Comments?

 

[Richard Blewett [DevelopMentor]]

Thats why you should prefer control.BeginInvoke over control.Invoke

Regards

Richard Blewett - DevelopMentor
http://www.dotnetconsult.co.uk/weblog
http://www.dotnetconsult.co.uk

I have a log class with static methods that grab and release a mutex as
required. The log class is designed to be called from both GUI and
thread pool threads, and dump output to a logfile and a RichEditBox.
To give you some idea of the approach I took...

private static RichTextBox rtb;
private delegate void RtbAppendTextHandler(string output);
private static RtbAppendTextHandler RtbAppendText;

Log.Init(RichTextBox richTextBox, string logFile)
{
rtb = richTextBox;
RtbAppendText = new RtbAppendTextHandler(rtb.AppendText);
Log.CreateLogFile(logFile);
}

Log.Write(string msg)
{
mutex.WaitOne();
string output = DateTime.Now.ToString() + " : " + msg;
rtb.Invoke(RtbAppendText, new object[] {output});
if(logToFile)
Log.WriteLogFile(output);
mutex.ReleaseMutex();
}

What appears to be happening is a worker thread is grabbing the mutex
and before the rtb.Invoke call the GUI thread waits on the mutex and
there things lockup. Which I take makes some sense, even though it
isn't what I'd like to happen. What I'm unsure of is how to best resolve
this issue.

The only place I manipulate the RichTextBox is in Log methods which
first wait for the mutex. Based on some things I've read, I got the
impression that that alone isn't enough to assure thread safety and when
one wants to manipulate a control from a worker thread they must do so
via Control.[Begin]Invoke. However, for fun, I tried changing the
rtb.Invoke(...) to an rtb.AppendText(output) and hitting the Log with
threads that write at a pretty good clip. Everything worked OK. I
then commented out the lines that wait/release on the mutex and tried
again and quickly got an exception. This leaves me wondering if the
mutex does in fact provide adequate protection. Comments?

[microsoft.public.dotnet.languages.csharp]

 

[Jon Skeet [C# MVP]]

I have a log class with static methods that grab and release a mutex as
required. The log class is designed to be called from both GUI and
thread pool threads, and dump output to a logfile and a RichEditBox.
To give you some idea of the approach I took...

Firstly, it's generally better to use Monitor.Enter/Exit than Mutexes
in .NET - they're cheaper, and the code can be more readable using the
"lock" statement in C#.

In this case you could either use BeginInvoke as Richard suggests, or
make sure you only lock when you absolutely need to. There's no
indication in your code that you particularly have to lock the mutex
until you need to call Log.WriteLogFile. That way you can make sure
that when you call Invoke, you don't hold the lock.

 

[User N]

Firstly, it's generally better to use Monitor.Enter/Exit than Mutexes
in .NET - they're cheaper, and the code can be more readable using the
"lock" statement in C#.

In this case you could either use BeginInvoke as Richard suggests, or
make sure you only lock when you absolutely need to. There's no
indication in your code that you particularly have to lock the mutex
until you need to call Log.WriteLogFile. That way you can make sure
that when you call Invoke, you don't hold the lock.

Thanks for the replies. The code I posted was merely meant to illustrate
the deadlock issue. I'm actually grabbing the mutex in one method
and releasing it in another, thus lock( ) was out. However, Monitor
Enter/Exit seem to work and if they are cheaper so be it.

A simple Invoke -> BeginInvoke solved the deadlock problem. I am,
however, finding that a secondary thread pounding the RichTextBox
with Invokes or BeginInvokes that directly or indirectly AppendText
brings the main form to its knees :-(

 

[Jon Skeet [C# MVP]]

Thanks for the replies. The code I posted was merely meant to illustrate
the deadlock issue. I'm actually grabbing the mutex in one method
and releasing it in another, thus lock( ) was out. However, Monitor
Enter/Exit seem to work and if they are cheaper so be it.

Acquiring a lock in one method and releasing it in another is almost
always a bad idea, just because it's so error prone. Do you absolutely
have to do it that way?

A simple Invoke -> BeginInvoke solved the deadlock problem. I am,
however, finding that a secondary thread pounding the RichTextBox
with Invokes or BeginInvokes that directly or indirectly AppendText
brings the main form to its knees :-(

If you're currently updating the UI every time something happens, you
may want to try only doing it if you haven't updated in the last half
second, or something like that - but make sure you always do the last
update!

 

[User N]

Acquiring a lock in one method and releasing it in another is almost
always a bad idea, just because it's so error prone. Do you absolutely
have to do it that way?

No, I really don't have to do it that way. It was a snap decision made
during rapid prototyping. I chose to support static usage such as this:

if(Log.Begin(LogMask.Errors))
Log.End("Foobar() failed");
...
if(Log.Begin(LogMask.ServerResponses))
{
Log.Write("Received server response:\n");
for(int i=0; i<rsp.LineCount; i++)
Log.Write(rsp.Lines);
Log.End();
}

If you're currently updating the UI every time something happens, you
may want to try only doing it if you haven't updated in the last half
second, or something like that - but make sure you always do the last
update!

Yeah, the idea was to do an AppendText for each log entry, in part
because it would be easy to spit each entry out in an appropriate color.
Reducing the number of appends by increasing the size of the appends
should help. I need to do some more benchmarking of the RichTextBox
though. Initial tests suggest the RichTextBox may not cut it with massive
amounts of log output, no matter what tricks one tries.