I believe that the attached patch solves this issue for both 0.8 and
0.10. If we stop the dispatcher thread before closing the session it
should not acquire a lock on _messageDeliveryLock. This may be useful to
apply it on M2.1.1.
Arnaud
On Mon, 2008-03-03 at 08:15 -0500, Rafael Schloming wrote:
> Aidan Skinner wrote:
> > On Fri, Feb 29, 2008 at 8:23 PM, Rafael Schloming <[EMAIL PROTECTED]> wrote:
> >
> >> The deadlock is between the session's _messageDeliveryLock and the
> >> Dispatcher's _lock. The dispatcher thread's main loop attempts to
> >> acquire _lock first and then _messageDeliveryLock. The main thread
> >> acquires _messageDeliveryLock first thing on close(...) and then
> >> subsequently many levels down the stack attempts to acquire _lock in
> >> order to call Dispatcher.rejectPending(...).
> >
> > We found (and fixed) several similar problems on M2, it's probably
> > worth having a look at that code to make sure there aren't any cases
> > being missed.
>
> I've just examined the M2.1 code, and I think the potential for the same
> deadlock exists there:
>
> AMQSession.close(long timeout) acquires _messageDeliveryLock and then
> indirectly calls BasicMessageConsumer.close().
>
> BasicMessageConsumer.close() calls syncWrite(BasicCancelOkBody.class).
> This will block until AMQSession.confirmConsumerCancelled() completes in
> another thread.
>
> If the dispatcher thread happens to be holding _lock and waiting to
> acquire _messageDeliveryLock, the confirmConsumerCancelled() in the
> other thread will never be able to acquire _lock and the same deadlock
> should occur.
>
> --Rafael
>
Index: org/apache/qpid/client/AMQSession.java
===================================================================
--- org/apache/qpid/client/AMQSession.java (revision 632970)
+++ org/apache/qpid/client/AMQSession.java (working copy)
@@ -459,6 +459,10 @@
// + Arrays.asList(Thread.currentThread().getStackTrace()).subList(3, 6));
}
+ if( _dispatcher != null )
+ {
+ _dispatcher.setConnectionStopped(true);
+ }
synchronized (_messageDeliveryLock)
{
