Ayende @ Rahien

Refunds available at head office

More disposal subtleties and framework bugs that stalks me

This one was a real pain to figure out. Can you imagine what would be the result of this code?

   1: var queue = new MessageQueue(queuePath);
   2: queue.Dispose();
   3: var peekAsyncResult = queue.BeginPeek();
   4: peekAsyncResult.AsyncWaitHandle.WaitOne();

If you guessed that we would get ObjectDisposedException, you are sadly mistaken. If you guessed that this would lead to a deadlock, you won.

Figuring out the behavior in a multi threaded system where one thread was beginning to listen and another was disposing the queue and waiting for pending operations to complete is… not fun.

Update: For some strange reason, I am not able to reproduce the problem shown above. I know that I did before I posted this, but I posted it as one of the last things that I did that day. I think that this is somehow related to the actual queue used and whatever or not it has messages.

Comments

Chris Patterson
01/24/2009 02:19 AM by
Chris Patterson

Is the Dispose supposed to be after the BeginPeek in your example?

And I've also seen some really strange behavior with MessageQueue and asynchronous operations. Probably has to do with all the COM glue that .NET goes through to talk to MSMQ. I remember having to go into reflector to figure it out one time.

smhinsey
01/24/2009 03:11 AM by
smhinsey

msmq can definitely be a headache with this stuff. i address this by building into the system the assumption that processing on a queue will not stop until the current message(s) are finished processing. i'm not sure that i understand your explanation enough to say that you're in the same situation.

Tran Ngoc Van
01/24/2009 03:57 AM by
Tran Ngoc Van

Your code is not supposed to use any object after it's disposed. It's true that ObjectDisposedException should be thrown, but sometimes developer forget to put a check in methods.

I have never had any check for disposed state in my classes and in my team, everyone have to make sure not to use a disposed object, mostly using IsDisposed property.

Most of the time I will unsubscribe from object's events before it's disposed to make sure that instance will not give me a surprise"tada".

Ayende Rahien
01/24/2009 04:08 AM by
Ayende Rahien

Chris,

No, dispose is called BEFORE BeginPeek.

The problem is that it happens in two different threads

Tran,

That is not always possible in a multi threaded applications.

smhinsey
01/24/2009 05:14 AM by
smhinsey

the way i avoided this was to adopt a threading model where one thread owns a queue and when you want to stop processing it, you send it a contol message, so you never directly interact with a queue from a thread that doesn't own it.

Ayende Rahien
01/24/2009 05:58 AM by
Ayende Rahien

Smhinsey,

It doesn't work like that when you want to be able to process messages from the queue on multiple threads.

smhinsey
01/24/2009 06:15 AM by
smhinsey

i do the actual message dispatch in a thread pool, so we spend very little time in the async callbacks.

Ayende Rahien
01/24/2009 06:21 AM by
Ayende Rahien

The problem isn't the message handling.

The problem is code like this (gmail code):

public void RegisterForMessageProcessing()

{

queue.BeginPeek( ar =>

{

var msg = queue.EndPeek(ar);

RegisterForMessageProcessing();

// do something with message

});

)

It is possible to get into a situation where you call BeginPeek and it never returns.

That is a problem

smhinsey
01/24/2009 06:33 AM by
smhinsey

ah, i never do a begin* without a timeout when i am dealing with msmq. i tend to also keep them very low.

smhinsey
01/24/2009 06:34 AM by
smhinsey

or are you saying it would still fail to return? if that's the case, i wish i could say i was shocked. i feel like system.messaging has probably not had the attention paid to it that it should with the ascent of wcf.

Ayende Rahien
01/24/2009 06:43 AM by
Ayende Rahien

I said that this is gmail code.

Tran Ngoc Van
01/24/2009 06:47 AM by
Tran Ngoc Van

The problem is you still need to re-initialize the queue so that it can handle the peek command.

You expected the ObjectDisposedException to be thrown, so instead of catching the exception, check if the queue disposed.

The problem with the Disposing that I can see:

  • There's always a very high chance that disposed state is not always checked by classes.

  • Developers must be careful to call Dispose once, and only once, or nasty bugs will jump out.

So I think you should always check if the queue is disposed, just like you have to have nasty null checks.

Omer Mor
01/24/2009 07:27 AM by
Omer Mor

I too was bitten occasionally by the poor multi threading support of the MessageQueue class.

An idea popped to my mind now:

Why not wrap this class with another that provides better disposal handling? ReSharper has the "Generate Delegating Members" feature that can help wrapping easily.

firefly
01/24/2009 08:23 PM by
firefly

Wow this is ugly. I agree with Omer the only sensible way to deal with this is to wrap it in another class. Seem to me we have to provide our own synchronization before calling Dispose and other methods.

It's probably hard for you to reproduce this bug because, I think, it is a very rare case for a deadlock to happen.

Comments have been closed on this topic.