qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rafael Schloming <rafa...@redhat.com>
Subject Re: Intermittent Test Failures [was: Re: M2 - let us try another "final" build]
Date Mon, 24 Sep 2007 12:07:43 GMT
Robert Greig wrote:
> On 22/09/2007, Robert Greig <robert.j.greig@gmail.com> wrote:
>> I managed to get a threaddump and it shows yet another deadlock
>> involving the dispatcher. Details are attached to QPID-589.
> Looking at the deadlock, it occurs because during session close, it
> sends Basic.Cancel for each consumer, and the Basic.Cancel-Ok handler
> (on a separate thread) calls Dispatcher.rejectPending which in turn
> tries to acquire the dispatcher lock. Sadly the dispatcher lock is
> already held by dispatcher.run(). Dispatcher.run is trying to acquire
> the messageDeliveryLock, which is already held by the close method is
> AMQSession.
> I couldn't spot an obvious solution involving reordering of locks.
> However it did occur to me that it was not necessary to send a
> Basic.Cancel where we are about to close the entire session (AMQP
> channel).
> Does anyone disagree and think we have to send Basic.Cancel?
> I have committed a change to the M2 branch so that it does not send
> Basic.Cancel where the session is closing and so far on our continuous
> build there have been no test failures or deadlocks. If it turns out
> that someone knows why we must send Basic.Cancel then I will obviously
> back out that change.

Strictly speaking I think you do need to send a basic.cancel. Without 
sending a basic.cancel and getting confirmation that the cancel is 
complete the broker will still be attempting to transmit messages to a 
client when the close occurs. If this happens when there is active 
message flow then there will pending messages when the close occurs and 
depending on how a broker behaves, this could cause messages to be 
unnecessarily DLQed, or unnecessarily lost in the case of no-ack.


View raw message