Ayende @ Rahien

Hi!
My name is Ayende Rahien
Founder of Hibernating Rhinos LTD and RavenDB.
You can reach me by phone or email:

ayende@ayende.com

+972 52-548-6969

@

Posts: 5,947 | Comments: 44,540

filter by tags archive

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

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

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

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

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

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

Smhinsey,

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

smhinsey

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

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

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

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

I said that this is gmail code.

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

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

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

Comments have been closed on this topic.

FUTURE POSTS

No future posts left, oh my!

RECENT SERIES

  1. RavenDB Sharding (2):
    21 May 2015 - Adding a new shard to an existing cluster, the easy way
  2. The RavenDB Comic Strip (2):
    20 May 2015 - Part II – a team in trouble!
  3. Challenge (45):
    28 Apr 2015 - What is the meaning of this change?
  4. Interview question (2):
    30 Mar 2015 - fix the index
  5. Excerpts from the RavenDB Performance team report (20):
    20 Feb 2015 - Optimizing Compare – The circle of life (a post-mortem)
View all series

RECENT COMMENTS

Syndication

Main feed Feed Stats
Comments feed   Comments Feed Stats