+972 52-548-6969
More disposal subtleties and framework bugs that stalks me
time to read 2 min | 383 words
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
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.
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.
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".
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.
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.
Smhinsey,
It doesn't work like that when you want to be able to process messages from the queue on multiple threads.
i do the actual message dispatch in a thread pool, so we spend very little time in the async callbacks.
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);
// do something with message
});
)
It is possible to get into a situation where you call BeginPeek and it never returns.
That is a problem
ah, i never do a begin* without a timeout when i am dealing with msmq. i tend to also keep them very low.
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.
I said that this is gmail code.
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.
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.
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.
Comment preview