Ayende @ Rahien

Refunds available at head office

I knew there were reasons for those multi threading warning lists

One of the warning signs for bad multi threaded code is calling code that you don’t own while holding a lock. I just discovered a bug in NH Prof that is related to just that issue.

But I wasn’t really stupid, and the story of this bug is a pretty interesting one.

Let us look at what happened. I have a deadlock in the application, where two threads are waiting for one another. In the UI thread, I have:

image

And on a background thread, I have:

image

This is actually code that happen in two separate classes. And it sounded very strange, until I walked up the stack and found:

image

All nice and tidy, and it looks good. Except that Sessions.Add invoke the SessionsCollectionChanged, which perform a block UI call, while the UI is trying to enter a lock that is currently being held.

Like I said, this is a classic threading issue, but it is pretty hard to see, unless you can actually get the stack trace.

Comments

tobi
09/19/2009 01:02 AM by
tobi

so the line between 3rd party code and own, but unrelated, code is very blurry. this was an interesting example for that.

Mike Rettig
09/19/2009 03:39 AM by
Mike Rettig

Oren,

Locking on shared state? I thought you were a proponent of message based concurrency. This post demonstrates exactly why concurrency combined with shared state is so hard.

Looking forward to your next thread race or deadlock,

Mike Rettig

Ayende Rahien
09/19/2009 11:37 AM by
Ayende Rahien

Mike,

I am. This is a case where I am updating the UI, I had to do something there.

John Chapman
09/19/2009 05:24 PM by
John Chapman

I don't know that I agree with your warning sign here. I think there are plenty of other areas of interest to discuss before talking about third party code and locks.

First, I'm guessing the issue is that the Execute.OnUIThread runs synchronously, meaning it does not return until the item has been executed on the UI thread? I would guess that would be the first warning sign. If that update could be queued up instead (ran asynchronously to the caller, this issue would have been averted).

I'm also a proponent of having certain classes run exclusively on the UI thread, (presenter/viewmodel/view), with the back end running on the whatever threads. Whenever code is executed on the presenter/viewmodel/view it is the UI thread (you ensure that through the semantics of your communication between UI and service layer). You then send your data in an immutable fashion to the UI thread such that the need for locking on the UI thread pretty much goes away. The code that is executing is always accessed from a single thread, so all of it's objects are free to touch, and all objects passed to these classes would be immutable (messages really) meaning they too can be accessed without working about locking.

That all being said there are a number of lessons to be learned from this example when you abstract out the UI thread part. This same issue can arise even without a UI.

Thanks for the story.

Comments have been closed on this topic.