You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@qpid.apache.org by Helen Kwong <he...@gmail.com> on 2015/02/04 23:36:37 UTC

Qpid Java client deadlocks

Hi Qpid gurus,

We are using 0.16 Java broker and client on 0-10, and we are running into
deadlock issues on the client that involve AMQSession's
_messageDeliveryLock and AMQConnection's _failoverMutex, where different
threads acquire them in different orders. This is leading to major
production headaches for us and has us very worried.

(I've been looking at 0.16 code mostly but also skimmed the relevant parts
in 0.32, which seem largely the same in those places.)

Deadlock Variety 1

This is an example of a deadlock we see, where the IOReceiver thread
deadlocks with Session dispatcher thread (we have listeners that call
session rollback or commit in onMessage()):

IOReceiver
org.apache.qpid.client.AMQSession.closed(AMQSession.java:818)  ---------->
waiting for session's messageDeliveryLock
org.apache.qpid.client.AMQConnection.closeAllSessions(AMQConnection.java:938)
org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1282)
 ----------> acquires connection's failoverMutex
org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
org.apache.qpid.client.AMQSession_0_10.exception(AMQSession_0_10.java:907)
org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:182)
org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:32)
org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:55)
org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:50)
org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:32)
org.apache.qpid.transport.Method.delegate(Method.java:159)
org.apache.qpid.transport.Session.received(Session.java:585)
org.apache.qpid.transport.Connection.dispatch(Connection.java:412)
org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:64)
org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:40)
org.apache.qpid.transport.MethodDelegate.executionException(MethodDelegate.java:110)
org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:54)
org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:40)
org.apache.qpid.transport.Method.delegate(Method.java:159)
org.apache.qpid.transport.Connection.received(Connection.java:367)
org.apache.qpid.transport.Connection.received(Connection.java:65)
org.apache.qpid.transport.network.Assembler.emit(Assembler.java:97)
org.apache.qpid.transport.network.Assembler.assemble(Assembler.java:198)
org.apache.qpid.transport.network.Assembler.frame(Assembler.java:131)
org.apache.qpid.transport.network.Frame.delegate(Frame.java:128)
org.apache.qpid.transport.network.Assembler.received(Assembler.java:102)
org.apache.qpid.transport.network.Assembler.received(Assembler.java:44)
org.apache.qpid.transport.network.InputHandler.next(InputHandler.java:189)
org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:105)
org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:44)
org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:152)
java.lang.Thread.run(Thread.java:745)

Session dispatcher thread
org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1255)
 ---------> waiting for connection's failoverMutex
org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
org.apache.qpid.client.AMQSession_0_10.sync(AMQSession_0_10.java:1034)
org.apache.qpid.client.AMQSession_0_10.sendSuspendChannel(AMQSession_0_10.java:812)
org.apache.qpid.client.AMQSession.suspendChannel(AMQSession.java:3075)
org.apache.qpid.client.AMQSession.rollback(AMQSession.java:1837)
common.messaging.QpidSession.rollback(QpidSession.java:211)
common.messaging.QpidMessageHandler.rollbackSession(QpidMessageHandler.java:284)
common.messaging.QpidMessageHandler.onMessage(QpidMessageHandler.java:113)
org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:748)
org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:141)
org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:722)
org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:186)
org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:54)
org.apache.qpid.client.AMQSession$Dispatcher.notifyConsumer(AMQSession.java:3454)
org.apache.qpid.client.AMQSession$Dispatcher.dispatchMessage(AMQSession.java:3393)
  -----------> acquires session's messageDeliverylock
org.apache.qpid.client.AMQSession$Dispatcher.access$1000(AMQSession.java:3180)
org.apache.qpid.client.AMQSession.dispatch(AMQSession.java:3173)
org.apache.qpid.client.message.UnprocessedMessage.dispatch(UnprocessedMessage.java:54)
org.apache.qpid.client.AMQSession$Dispatcher.run(AMQSession.java:3316)
java.lang.Thread.run(Thread.java:745)

The problem is that the IOReceiver thread acquires failoverMutex before
messageDeliveryLock (for each session), whereas the dispatcher thread
acquires it in the other order. We also see potential problems where other
threads (instead of IOReceiver) can deadlock with the dispatcher thread, as
long as it acquires failoverMutex before messageDeliveryLock. Examples we
can think of:

A) Another thread calling AMQSession.close()
B) Another thread calling BasicMessageConsumer.close()
C) Same connection, different session's dispatcher thread, calling
rollback() or commit() -> sync() -> setCurrentException() ->
AMQConnection.exceptionReceived() -> AMQConnection.closeAllSessions(),
which can try to acquire the messageDeliveryLock of another session and
deadlock with the other session's dispatcher thread


Deadlock Variety 2:
>From code inspection, it also appears that AMQConnection.close() can
deadlock with either AMQSession.close() or BasicMessageConsumer.close()
(where the session / consumer is on the same connection). This is because
AMQConnection.close() first acquires the messageDeliveryLock of all its
sessions in the recursive doClose(), before trying to acquire the
connection's failoverMutex. But the Session / consumer's close() acquires
the failoverMutex before messageDeliveryLock. We haven't seen this happen
but would like to know if this is possible.


We'd really appreciate your help on this. Assuming these can be fixed in
0.32, we are also wondering if clients are backward compatible -- i.e., can
we upgrade only our client to 0.32 while continuing to use the 0.16 broker?

Thanks,
Helen

Re: Qpid Java client deadlocks

Posted by Rob Godfrey <ro...@gmail.com>.
>From my point of view, I'm thinking that if an isolated change - such as
the one I am implicitly proposing - makes things *better* (in this case
solving the issue that Helen is seeing) and doesn't obviously introduce any
new issues, then it appears worthwhile.  Trying to reengineer the client to
guarantee there are never any deadlocks is more than I would want to take
on.

Of the existing JIRAs which I can see are about deadlocks, I think the
following is true:

QPID-3207 : AMQSession.java close() does not close the consumers with the
correct arguments
  the commit here was rolled back when it caused deadlocks.  I'm not sure
what the problem statement is here, but I suggest we close "will not fix"
this JIRA

QPID-3216 : Deadlock between "_lock" (in AMQSession#DisptcherThread) and
"_messageDeliveryLock" (in AMQSession.java)
  this issue only occurred when the (reverted) change from QPID-3207 was
applied.  As is, nothing that takes the _messageDeliveryLock should
subsequently take _lock, so no deadlock here will occur.

QPID-3259 : Deadlock on Java client side while closing session when topic
operation is unauthorized
  issue where a synchronous operation times out while blocked against the
server generated close (this issue is not addressed by my changes)

QPID-4276 : Java client - deadlock on concurrent close of consumer and
connection
  this issue should be fixed by the proposed change (occurs because
consumer close acquired MDL and FM in opposite order to connection close)

QPID-4574 : [JMS] Deadlock involving _failoverMutex and _messageDeliveryLock
  this is should be fixed by the proposed change (again issue that MDL and
FM were acquired in the opposite order)

QPID-4906 : If Session close() or closed() method is invoked while inside
onMessage(), they should be excuted after onMessage() has completed.
  close() behaviour is fixed by the proposed change.  closed() no longer
acquires MDL, and shouldn't block.

QPID-4922 : If Consumer close() method is invoked while inside onMessage(),
it should be excuted after onMessage() has completed.
  The title of this seems incorrect.
  Moreover after the proposed change I do not believe a deadlock should be
possible as long as the consumer being closed in on the same session.
  Closing a consumer on a different session of the same connection may
cause issues.

QPID-5117 : Java client deadlocks if connection is closed while onMessage()
is creating a session
  This should be fixed after my proposed change

QPID-5118 : [JMS] Deadlock when closing a consumer and calling JMS
operatons inside onMessage() on AMQP 0-8/0-9/0-9-1 connection
  This should be fixed after my proposed change

QPID-5119 : [JMS] 3-thread deadlock on pre-AMQP-0-10 connection involving
messageDeliveryLock, failover mutex and sessionCreationLock
  This should be fixed by my proposed change

If anyone knows  any other JIRAs out there regarding deadlocks it'd be good
to look at those too...


-- Rob

On 9 February 2015 at 15:55, Rajith Muditha Attapattu <ra...@gmail.com>
wrote:

> On Mon, Feb 9, 2015 at 6:15 AM, Robbie Gemmell <ro...@gmail.com>
> wrote:
>
> > The main thing I recall from back then is it being hard to
> > reason about the impact of such changes and the interaction of the
> > various locks due to the way the client threads can enter the
> > application space (e.g onMessage, exception listeners) and then
> > re-enter the client code while holding an unclear set of locks.
> >
>
>
> This was the main reason most of the proposed changes weren't applied.
> All though the existing tests were passing, concerns were raised about not
> being able to measure the impact.
> It was a legitimate concern.
> I simply gave up after that and took the alternate approach to helping one
> of our customers who could no longer use the qpid client.
>
> It's great if someone could fix these issues, but I no longer have the
> desire nor the bandwidth to look at them again.
>
> Rajith
>

Re: Qpid Java client deadlocks

Posted by Rajith Muditha Attapattu <ra...@gmail.com>.
On Mon, Feb 9, 2015 at 6:15 AM, Robbie Gemmell <ro...@gmail.com>
wrote:

> The main thing I recall from back then is it being hard to
> reason about the impact of such changes and the interaction of the
> various locks due to the way the client threads can enter the
> application space (e.g onMessage, exception listeners) and then
> re-enter the client code while holding an unclear set of locks.
>


This was the main reason most of the proposed changes weren't applied.
All though the existing tests were passing, concerns were raised about not
being able to measure the impact.
It was a legitimate concern.
I simply gave up after that and took the alternate approach to helping one
of our customers who could no longer use the qpid client.

It's great if someone could fix these issues, but I no longer have the
desire nor the bandwidth to look at them again.

Rajith

Re: Qpid Java client deadlocks

Posted by Robbie Gemmell <ro...@gmail.com>.
Sounds reasonable.

Robbie

On 9 February 2015 at 13:40, Rob Godfrey <ro...@gmail.com> wrote:
> So looking back to when the MDL acquisition in closed() was first
> introduced... it seems to have been for
> https://issues.apache.org/jira/browse/QPID-547 which I think really should
> only have covered the close() case (i.e. application initiated closure)
> rather than the server initiated closure.
>
> As such I'm happy removing the MDL acquisition there.
>
> -- Rob
>
> On 9 February 2015 at 13:26, Robbie Gemmell <ro...@gmail.com>
> wrote:
>
>> Taking a quick peek it as you said *seems* safe, since the 'closed'
>> call looks like the only place a non-app non-dispatcher thread could
>> get the MDL. I would probably need a long time to really convince
>> myself though.
>>
>> Robbie
>>
>> On 9 February 2015 at 11:47, Rob Godfrey <ro...@gmail.com> wrote:
>> > Hi Robbie,
>> >
>> > yes - my thinking is currently that the acquiring of the MDL in
>> > AMQSession.closed() is incorrect.  The MDL seems to be there to provide
>> JMS
>> > semantics around the interaction between message delivery and other
>> methods
>> > on the (JMS) session object.  As such it should only really be acquired
>> > from threads acting on behalf of the "application".  Application
>> initiated
>> > closes already hold the MDL by the time they get into AMQSession.closed()
>> > so I think this can safely be removed.  In the other cases of the MDL
>> being
>> > acquired after the failover mutex (such as BasicMessageConsumer.close())
>> I
>> > can see no reason why the lock acquisition order cannot be reversed (the
>> > locks are pretty much taken out one after another) and by reversing them
>> we
>> > can prevent deadlock with the Connection close().
>> >
>> > Making the above change *seems* safe to me and (for what little it is
>> > worth) all the system tests continue to pass without issue on both the
>> > 0-9-1 and 0-10 paths.
>> >
>> > Can anyone see any issues with the above?
>> >
>> > -- Rob
>> >
>> > On 9 February 2015 at 12:15, Robbie Gemmell <ro...@gmail.com>
>> > wrote:
>> >
>> >> Hi Rob,
>> >>
>> >> Its a couple of years since I last did any real work on the 0-10
>> >> client, but around that time I did review some proposed changes
>> >> attempting removal of the MDL enough to know they likely introduced
>> >> new issues. The main thing I recall from back then is it being hard to
>> >> reason about the impact of such changes and the interaction of the
>> >> various locks due to the way the client threads can enter the
>> >> application space (e.g onMessage, exception listeners) and then
>> >> re-enter the client code while holding an unclear set of locks.
>> >>
>> >> What you wrote certainly makes logical sense in that if you always
>> >> acquire the MDL before the FM then even that behaviour shouldnt then
>> >> be an issue in terms of deadlocking between those two. The main
>> >> weirdness I can think comes of in terms of the stacktrace given in
>> >> that if the dispatcher is inside onMessage holding the MDL it could
>> >> then block the connection IOReceiver thread from completing whatever
>> >> it is attempting, leaving it unable to service new bytes arriving from
>> >> the broker (e.g should anything onMessage proceeds to do actually
>> >> generate new requests, or there being other sessions) as well as doing
>> >> anything else it would normally do, until that sessions onMessage
>> >> exists, which may itself cause different problems. Looking again at
>> >> the the given stacktrace, it does sound reasonable for the lock order
>> >> in connection close to be reversed, and I also wonder if the MDL is
>> >> truely needed when indicating the session closed due to an execution
>> >> exception. My hands are currently full on other things though so these
>> >> aren't areas I am going to be able to look at personally.
>> >>
>> >> Robbie
>> >>
>> >> On 7 February 2015 at 14:23, Rob Godfrey <ro...@gmail.com>
>> wrote:
>> >> > Hi Rajith,
>> >> >
>> >> > so - I'm no expert on the locking model in the client... Logically I
>> >> would
>> >> > think that with the message delivery lock (MDL) being a session level
>> >> lock,
>> >> > and the failover mutex (FM) being connection wide, we should always
>> >> acquire
>> >> > the MDL before the FM.  Where we are currently acquiring the FM
>> without
>> >> > acquiring the MDL I would (probably naively) think that we should
>> first
>> >> be
>> >> > acquiring the MDL before the FM where the operation in question is
>> >> specific
>> >> > to a session (e.g. send a message, declare a queue, etc).  In the case
>> >> > where the operation is connection wide (which I think is pretty much
>> >> > limitted to connection closure) I would think that we would want to
>> get
>> >> the
>> >> > MDLs for all of the sessions on the connection before getting the FM.
>> >> >
>> >> > Looking at the code I couldn't immediately see why such a change would
>> >> > cause an issue - if something is performing an operation against a
>> >> session
>> >> > in onMessage mode then it would currently hold the MDL... and if we
>> are
>> >> > using receive() then holding the MDL is seemingly compatible with the
>> >> > definition of given by the javadoc.
>> >> >
>> >> > I expect I'm missing some subtlety here - can someone enlighten me?
>> >> >
>> >> > Thanks,
>> >> > Rob
>> >> >
>> >> > On 5 February 2015 at 15:16, Rajith Muditha Attapattu <
>> >> rajith77@gmail.com>
>> >> > wrote:
>> >> >
>> >> >> The consensus is, that there is no easy fix, other than rewriting
>> some
>> >> key
>> >> >> pieces of the client.
>> >> >> Given that a new JMS client is being actively developed, we have
>> decided
>> >> >> not to devote any significant time/resources on the older client.
>> >> >>
>> >> >> The new JMS client is AMQP 1.0 and the source is here
>> >> >> https://git-wip-us.apache.org/repos/asf?p=qpid-jms.git
>> >> >> Robbie is the best person to give an accurate picture of the status
>> for
>> >> >> this project.
>> >> >>
>> >> >> Rajith
>> >> >>
>> >> >> On Thu, Feb 5, 2015 at 2:42 AM, Rob Godfrey <rob.j.godfrey@gmail.com
>> >
>> >> >> wrote:
>> >> >>
>> >> >> > To answer the easy question first, 0.32 clients should be totally
>> >> >> > compatible with an 0.16 broker.
>> >> >> >
>> >> >> > In terms of the deadlock issue, I know there are a number of open
>> >> >> deadlock
>> >> >> > JIRAs that Robbie and Rajith have looked at in the past...
>> Hopefully
>> >> one
>> >> >> of
>> >> >> > them will be able to chip in here and discuss the feasibility of
>> >> fixing
>> >> >> > your particular issue for 0.32.
>> >> >> >
>> >> >> > -- Rob
>> >> >> >
>> >> >> > On 4 February 2015 at 23:36, Helen Kwong <he...@gmail.com>
>> >> wrote:
>> >> >> >
>> >> >> > > Hi Qpid gurus,
>> >> >> > >
>> >> >> > > We are using 0.16 Java broker and client on 0-10, and we are
>> running
>> >> >> into
>> >> >> > > deadlock issues on the client that involve AMQSession's
>> >> >> > > _messageDeliveryLock and AMQConnection's _failoverMutex, where
>> >> >> different
>> >> >> > > threads acquire them in different orders. This is leading to
>> major
>> >> >> > > production headaches for us and has us very worried.
>> >> >> > >
>> >> >> > > (I've been looking at 0.16 code mostly but also skimmed the
>> relevant
>> >> >> > parts
>> >> >> > > in 0.32, which seem largely the same in those places.)
>> >> >> > >
>> >> >> > > Deadlock Variety 1
>> >> >> > >
>> >> >> > > This is an example of a deadlock we see, where the IOReceiver
>> thread
>> >> >> > > deadlocks with Session dispatcher thread (we have listeners that
>> >> call
>> >> >> > > session rollback or commit in onMessage()):
>> >> >> > >
>> >> >> > > IOReceiver
>> >> >> > > org.apache.qpid.client.AMQSession.closed(AMQSession.java:818)
>> >> >> > ---------->
>> >> >> > > waiting for session's messageDeliveryLock
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.client.AMQConnection.closeAllSessions(AMQConnection.java:938)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1282)
>> >> >> > >  ----------> acquires connection's failoverMutex
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.client.AMQSession_0_10.exception(AMQSession_0_10.java:907)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:182)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:32)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:55)
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:50)
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:32)
>> >> >> > > org.apache.qpid.transport.Method.delegate(Method.java:159)
>> >> >> > > org.apache.qpid.transport.Session.received(Session.java:585)
>> >> >> > >
>> org.apache.qpid.transport.Connection.dispatch(Connection.java:412)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:64)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:40)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.transport.MethodDelegate.executionException(MethodDelegate.java:110)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:54)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:40)
>> >> >> > > org.apache.qpid.transport.Method.delegate(Method.java:159)
>> >> >> > >
>> org.apache.qpid.transport.Connection.received(Connection.java:367)
>> >> >> > > org.apache.qpid.transport.Connection.received(Connection.java:65)
>> >> >> > >
>> org.apache.qpid.transport.network.Assembler.emit(Assembler.java:97)
>> >> >> > >
>> >> >>
>> org.apache.qpid.transport.network.Assembler.assemble(Assembler.java:198)
>> >> >> > >
>> >> org.apache.qpid.transport.network.Assembler.frame(Assembler.java:131)
>> >> >> > > org.apache.qpid.transport.network.Frame.delegate(Frame.java:128)
>> >> >> > >
>> >> >>
>> org.apache.qpid.transport.network.Assembler.received(Assembler.java:102)
>> >> >> > >
>> >> org.apache.qpid.transport.network.Assembler.received(Assembler.java:44)
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.transport.network.InputHandler.next(InputHandler.java:189)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:105)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:44)
>> >> >> > >
>> >> >>
>> org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:152)
>> >> >> > > java.lang.Thread.run(Thread.java:745)
>> >> >> > >
>> >> >> > > Session dispatcher thread
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1255)
>> >> >> > >  ---------> waiting for connection's failoverMutex
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
>> >> >> > >
>> >> org.apache.qpid.client.AMQSession_0_10.sync(AMQSession_0_10.java:1034)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.client.AMQSession_0_10.sendSuspendChannel(AMQSession_0_10.java:812)
>> >> >> > >
>> >> org.apache.qpid.client.AMQSession.suspendChannel(AMQSession.java:3075)
>> >> >> > > org.apache.qpid.client.AMQSession.rollback(AMQSession.java:1837)
>> >> >> > > common.messaging.QpidSession.rollback(QpidSession.java:211)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> common.messaging.QpidMessageHandler.rollbackSession(QpidMessageHandler.java:284)
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> common.messaging.QpidMessageHandler.onMessage(QpidMessageHandler.java:113)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:748)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:141)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:722)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:186)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:54)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.client.AMQSession$Dispatcher.notifyConsumer(AMQSession.java:3454)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.client.AMQSession$Dispatcher.dispatchMessage(AMQSession.java:3393)
>> >> >> > >   -----------> acquires session's messageDeliverylock
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.client.AMQSession$Dispatcher.access$1000(AMQSession.java:3180)
>> >> >> > > org.apache.qpid.client.AMQSession.dispatch(AMQSession.java:3173)
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> org.apache.qpid.client.message.UnprocessedMessage.dispatch(UnprocessedMessage.java:54)
>> >> >> > >
>> >> org.apache.qpid.client.AMQSession$Dispatcher.run(AMQSession.java:3316)
>> >> >> > > java.lang.Thread.run(Thread.java:745)
>> >> >> > >
>> >> >> > > The problem is that the IOReceiver thread acquires failoverMutex
>> >> before
>> >> >> > > messageDeliveryLock (for each session), whereas the dispatcher
>> >> thread
>> >> >> > > acquires it in the other order. We also see potential problems
>> where
>> >> >> > other
>> >> >> > > threads (instead of IOReceiver) can deadlock with the dispatcher
>> >> >> thread,
>> >> >> > as
>> >> >> > > long as it acquires failoverMutex before messageDeliveryLock.
>> >> Examples
>> >> >> we
>> >> >> > > can think of:
>> >> >> > >
>> >> >> > > A) Another thread calling AMQSession.close()
>> >> >> > > B) Another thread calling BasicMessageConsumer.close()
>> >> >> > > C) Same connection, different session's dispatcher thread,
>> calling
>> >> >> > > rollback() or commit() -> sync() -> setCurrentException() ->
>> >> >> > > AMQConnection.exceptionReceived() ->
>> >> AMQConnection.closeAllSessions(),
>> >> >> > > which can try to acquire the messageDeliveryLock of another
>> session
>> >> and
>> >> >> > > deadlock with the other session's dispatcher thread
>> >> >> > >
>> >> >> > >
>> >> >> > > Deadlock Variety 2:
>> >> >> > > From code inspection, it also appears that AMQConnection.close()
>> can
>> >> >> > > deadlock with either AMQSession.close() or
>> >> BasicMessageConsumer.close()
>> >> >> > > (where the session / consumer is on the same connection). This is
>> >> >> because
>> >> >> > > AMQConnection.close() first acquires the messageDeliveryLock of
>> all
>> >> its
>> >> >> > > sessions in the recursive doClose(), before trying to acquire the
>> >> >> > > connection's failoverMutex. But the Session / consumer's close()
>> >> >> acquires
>> >> >> > > the failoverMutex before messageDeliveryLock. We haven't seen
>> this
>> >> >> happen
>> >> >> > > but would like to know if this is possible.
>> >> >> > >
>> >> >> > >
>> >> >> > > We'd really appreciate your help on this. Assuming these can be
>> >> fixed
>> >> >> in
>> >> >> > > 0.32, we are also wondering if clients are backward compatible --
>> >> i.e.,
>> >> >> > can
>> >> >> > > we upgrade only our client to 0.32 while continuing to use the
>> 0.16
>> >> >> > broker?
>> >> >> > >
>> >> >> > > Thanks,
>> >> >> > > Helen
>> >> >> > >
>> >> >> >
>> >> >>
>> >>
>> >> ---------------------------------------------------------------------
>> >> To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
>> >> For additional commands, e-mail: users-help@qpid.apache.org
>> >>
>> >>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
>> For additional commands, e-mail: users-help@qpid.apache.org
>>
>>

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org


Re: Qpid Java client deadlocks

Posted by Rob Godfrey <ro...@gmail.com>.
So looking back to when the MDL acquisition in closed() was first
introduced... it seems to have been for
https://issues.apache.org/jira/browse/QPID-547 which I think really should
only have covered the close() case (i.e. application initiated closure)
rather than the server initiated closure.

As such I'm happy removing the MDL acquisition there.

-- Rob

On 9 February 2015 at 13:26, Robbie Gemmell <ro...@gmail.com>
wrote:

> Taking a quick peek it as you said *seems* safe, since the 'closed'
> call looks like the only place a non-app non-dispatcher thread could
> get the MDL. I would probably need a long time to really convince
> myself though.
>
> Robbie
>
> On 9 February 2015 at 11:47, Rob Godfrey <ro...@gmail.com> wrote:
> > Hi Robbie,
> >
> > yes - my thinking is currently that the acquiring of the MDL in
> > AMQSession.closed() is incorrect.  The MDL seems to be there to provide
> JMS
> > semantics around the interaction between message delivery and other
> methods
> > on the (JMS) session object.  As such it should only really be acquired
> > from threads acting on behalf of the "application".  Application
> initiated
> > closes already hold the MDL by the time they get into AMQSession.closed()
> > so I think this can safely be removed.  In the other cases of the MDL
> being
> > acquired after the failover mutex (such as BasicMessageConsumer.close())
> I
> > can see no reason why the lock acquisition order cannot be reversed (the
> > locks are pretty much taken out one after another) and by reversing them
> we
> > can prevent deadlock with the Connection close().
> >
> > Making the above change *seems* safe to me and (for what little it is
> > worth) all the system tests continue to pass without issue on both the
> > 0-9-1 and 0-10 paths.
> >
> > Can anyone see any issues with the above?
> >
> > -- Rob
> >
> > On 9 February 2015 at 12:15, Robbie Gemmell <ro...@gmail.com>
> > wrote:
> >
> >> Hi Rob,
> >>
> >> Its a couple of years since I last did any real work on the 0-10
> >> client, but around that time I did review some proposed changes
> >> attempting removal of the MDL enough to know they likely introduced
> >> new issues. The main thing I recall from back then is it being hard to
> >> reason about the impact of such changes and the interaction of the
> >> various locks due to the way the client threads can enter the
> >> application space (e.g onMessage, exception listeners) and then
> >> re-enter the client code while holding an unclear set of locks.
> >>
> >> What you wrote certainly makes logical sense in that if you always
> >> acquire the MDL before the FM then even that behaviour shouldnt then
> >> be an issue in terms of deadlocking between those two. The main
> >> weirdness I can think comes of in terms of the stacktrace given in
> >> that if the dispatcher is inside onMessage holding the MDL it could
> >> then block the connection IOReceiver thread from completing whatever
> >> it is attempting, leaving it unable to service new bytes arriving from
> >> the broker (e.g should anything onMessage proceeds to do actually
> >> generate new requests, or there being other sessions) as well as doing
> >> anything else it would normally do, until that sessions onMessage
> >> exists, which may itself cause different problems. Looking again at
> >> the the given stacktrace, it does sound reasonable for the lock order
> >> in connection close to be reversed, and I also wonder if the MDL is
> >> truely needed when indicating the session closed due to an execution
> >> exception. My hands are currently full on other things though so these
> >> aren't areas I am going to be able to look at personally.
> >>
> >> Robbie
> >>
> >> On 7 February 2015 at 14:23, Rob Godfrey <ro...@gmail.com>
> wrote:
> >> > Hi Rajith,
> >> >
> >> > so - I'm no expert on the locking model in the client... Logically I
> >> would
> >> > think that with the message delivery lock (MDL) being a session level
> >> lock,
> >> > and the failover mutex (FM) being connection wide, we should always
> >> acquire
> >> > the MDL before the FM.  Where we are currently acquiring the FM
> without
> >> > acquiring the MDL I would (probably naively) think that we should
> first
> >> be
> >> > acquiring the MDL before the FM where the operation in question is
> >> specific
> >> > to a session (e.g. send a message, declare a queue, etc).  In the case
> >> > where the operation is connection wide (which I think is pretty much
> >> > limitted to connection closure) I would think that we would want to
> get
> >> the
> >> > MDLs for all of the sessions on the connection before getting the FM.
> >> >
> >> > Looking at the code I couldn't immediately see why such a change would
> >> > cause an issue - if something is performing an operation against a
> >> session
> >> > in onMessage mode then it would currently hold the MDL... and if we
> are
> >> > using receive() then holding the MDL is seemingly compatible with the
> >> > definition of given by the javadoc.
> >> >
> >> > I expect I'm missing some subtlety here - can someone enlighten me?
> >> >
> >> > Thanks,
> >> > Rob
> >> >
> >> > On 5 February 2015 at 15:16, Rajith Muditha Attapattu <
> >> rajith77@gmail.com>
> >> > wrote:
> >> >
> >> >> The consensus is, that there is no easy fix, other than rewriting
> some
> >> key
> >> >> pieces of the client.
> >> >> Given that a new JMS client is being actively developed, we have
> decided
> >> >> not to devote any significant time/resources on the older client.
> >> >>
> >> >> The new JMS client is AMQP 1.0 and the source is here
> >> >> https://git-wip-us.apache.org/repos/asf?p=qpid-jms.git
> >> >> Robbie is the best person to give an accurate picture of the status
> for
> >> >> this project.
> >> >>
> >> >> Rajith
> >> >>
> >> >> On Thu, Feb 5, 2015 at 2:42 AM, Rob Godfrey <rob.j.godfrey@gmail.com
> >
> >> >> wrote:
> >> >>
> >> >> > To answer the easy question first, 0.32 clients should be totally
> >> >> > compatible with an 0.16 broker.
> >> >> >
> >> >> > In terms of the deadlock issue, I know there are a number of open
> >> >> deadlock
> >> >> > JIRAs that Robbie and Rajith have looked at in the past...
> Hopefully
> >> one
> >> >> of
> >> >> > them will be able to chip in here and discuss the feasibility of
> >> fixing
> >> >> > your particular issue for 0.32.
> >> >> >
> >> >> > -- Rob
> >> >> >
> >> >> > On 4 February 2015 at 23:36, Helen Kwong <he...@gmail.com>
> >> wrote:
> >> >> >
> >> >> > > Hi Qpid gurus,
> >> >> > >
> >> >> > > We are using 0.16 Java broker and client on 0-10, and we are
> running
> >> >> into
> >> >> > > deadlock issues on the client that involve AMQSession's
> >> >> > > _messageDeliveryLock and AMQConnection's _failoverMutex, where
> >> >> different
> >> >> > > threads acquire them in different orders. This is leading to
> major
> >> >> > > production headaches for us and has us very worried.
> >> >> > >
> >> >> > > (I've been looking at 0.16 code mostly but also skimmed the
> relevant
> >> >> > parts
> >> >> > > in 0.32, which seem largely the same in those places.)
> >> >> > >
> >> >> > > Deadlock Variety 1
> >> >> > >
> >> >> > > This is an example of a deadlock we see, where the IOReceiver
> thread
> >> >> > > deadlocks with Session dispatcher thread (we have listeners that
> >> call
> >> >> > > session rollback or commit in onMessage()):
> >> >> > >
> >> >> > > IOReceiver
> >> >> > > org.apache.qpid.client.AMQSession.closed(AMQSession.java:818)
> >> >> > ---------->
> >> >> > > waiting for session's messageDeliveryLock
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.client.AMQConnection.closeAllSessions(AMQConnection.java:938)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1282)
> >> >> > >  ----------> acquires connection's failoverMutex
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.client.AMQSession_0_10.exception(AMQSession_0_10.java:907)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:182)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:32)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:55)
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:50)
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:32)
> >> >> > > org.apache.qpid.transport.Method.delegate(Method.java:159)
> >> >> > > org.apache.qpid.transport.Session.received(Session.java:585)
> >> >> > >
> org.apache.qpid.transport.Connection.dispatch(Connection.java:412)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:64)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:40)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.transport.MethodDelegate.executionException(MethodDelegate.java:110)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:54)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:40)
> >> >> > > org.apache.qpid.transport.Method.delegate(Method.java:159)
> >> >> > >
> org.apache.qpid.transport.Connection.received(Connection.java:367)
> >> >> > > org.apache.qpid.transport.Connection.received(Connection.java:65)
> >> >> > >
> org.apache.qpid.transport.network.Assembler.emit(Assembler.java:97)
> >> >> > >
> >> >>
> org.apache.qpid.transport.network.Assembler.assemble(Assembler.java:198)
> >> >> > >
> >> org.apache.qpid.transport.network.Assembler.frame(Assembler.java:131)
> >> >> > > org.apache.qpid.transport.network.Frame.delegate(Frame.java:128)
> >> >> > >
> >> >>
> org.apache.qpid.transport.network.Assembler.received(Assembler.java:102)
> >> >> > >
> >> org.apache.qpid.transport.network.Assembler.received(Assembler.java:44)
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.transport.network.InputHandler.next(InputHandler.java:189)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:105)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:44)
> >> >> > >
> >> >>
> org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:152)
> >> >> > > java.lang.Thread.run(Thread.java:745)
> >> >> > >
> >> >> > > Session dispatcher thread
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1255)
> >> >> > >  ---------> waiting for connection's failoverMutex
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
> >> >> > >
> >> org.apache.qpid.client.AMQSession_0_10.sync(AMQSession_0_10.java:1034)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.client.AMQSession_0_10.sendSuspendChannel(AMQSession_0_10.java:812)
> >> >> > >
> >> org.apache.qpid.client.AMQSession.suspendChannel(AMQSession.java:3075)
> >> >> > > org.apache.qpid.client.AMQSession.rollback(AMQSession.java:1837)
> >> >> > > common.messaging.QpidSession.rollback(QpidSession.java:211)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> common.messaging.QpidMessageHandler.rollbackSession(QpidMessageHandler.java:284)
> >> >> > >
> >> >> >
> >> >>
> >>
> common.messaging.QpidMessageHandler.onMessage(QpidMessageHandler.java:113)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:748)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:141)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:722)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:186)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:54)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.client.AMQSession$Dispatcher.notifyConsumer(AMQSession.java:3454)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.client.AMQSession$Dispatcher.dispatchMessage(AMQSession.java:3393)
> >> >> > >   -----------> acquires session's messageDeliverylock
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.client.AMQSession$Dispatcher.access$1000(AMQSession.java:3180)
> >> >> > > org.apache.qpid.client.AMQSession.dispatch(AMQSession.java:3173)
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >>
> org.apache.qpid.client.message.UnprocessedMessage.dispatch(UnprocessedMessage.java:54)
> >> >> > >
> >> org.apache.qpid.client.AMQSession$Dispatcher.run(AMQSession.java:3316)
> >> >> > > java.lang.Thread.run(Thread.java:745)
> >> >> > >
> >> >> > > The problem is that the IOReceiver thread acquires failoverMutex
> >> before
> >> >> > > messageDeliveryLock (for each session), whereas the dispatcher
> >> thread
> >> >> > > acquires it in the other order. We also see potential problems
> where
> >> >> > other
> >> >> > > threads (instead of IOReceiver) can deadlock with the dispatcher
> >> >> thread,
> >> >> > as
> >> >> > > long as it acquires failoverMutex before messageDeliveryLock.
> >> Examples
> >> >> we
> >> >> > > can think of:
> >> >> > >
> >> >> > > A) Another thread calling AMQSession.close()
> >> >> > > B) Another thread calling BasicMessageConsumer.close()
> >> >> > > C) Same connection, different session's dispatcher thread,
> calling
> >> >> > > rollback() or commit() -> sync() -> setCurrentException() ->
> >> >> > > AMQConnection.exceptionReceived() ->
> >> AMQConnection.closeAllSessions(),
> >> >> > > which can try to acquire the messageDeliveryLock of another
> session
> >> and
> >> >> > > deadlock with the other session's dispatcher thread
> >> >> > >
> >> >> > >
> >> >> > > Deadlock Variety 2:
> >> >> > > From code inspection, it also appears that AMQConnection.close()
> can
> >> >> > > deadlock with either AMQSession.close() or
> >> BasicMessageConsumer.close()
> >> >> > > (where the session / consumer is on the same connection). This is
> >> >> because
> >> >> > > AMQConnection.close() first acquires the messageDeliveryLock of
> all
> >> its
> >> >> > > sessions in the recursive doClose(), before trying to acquire the
> >> >> > > connection's failoverMutex. But the Session / consumer's close()
> >> >> acquires
> >> >> > > the failoverMutex before messageDeliveryLock. We haven't seen
> this
> >> >> happen
> >> >> > > but would like to know if this is possible.
> >> >> > >
> >> >> > >
> >> >> > > We'd really appreciate your help on this. Assuming these can be
> >> fixed
> >> >> in
> >> >> > > 0.32, we are also wondering if clients are backward compatible --
> >> i.e.,
> >> >> > can
> >> >> > > we upgrade only our client to 0.32 while continuing to use the
> 0.16
> >> >> > broker?
> >> >> > >
> >> >> > > Thanks,
> >> >> > > Helen
> >> >> > >
> >> >> >
> >> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
> >> For additional commands, e-mail: users-help@qpid.apache.org
> >>
> >>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
> For additional commands, e-mail: users-help@qpid.apache.org
>
>

Re: Qpid Java client deadlocks

Posted by Robbie Gemmell <ro...@gmail.com>.
Taking a quick peek it as you said *seems* safe, since the 'closed'
call looks like the only place a non-app non-dispatcher thread could
get the MDL. I would probably need a long time to really convince
myself though.

Robbie

On 9 February 2015 at 11:47, Rob Godfrey <ro...@gmail.com> wrote:
> Hi Robbie,
>
> yes - my thinking is currently that the acquiring of the MDL in
> AMQSession.closed() is incorrect.  The MDL seems to be there to provide JMS
> semantics around the interaction between message delivery and other methods
> on the (JMS) session object.  As such it should only really be acquired
> from threads acting on behalf of the "application".  Application initiated
> closes already hold the MDL by the time they get into AMQSession.closed()
> so I think this can safely be removed.  In the other cases of the MDL being
> acquired after the failover mutex (such as BasicMessageConsumer.close()) I
> can see no reason why the lock acquisition order cannot be reversed (the
> locks are pretty much taken out one after another) and by reversing them we
> can prevent deadlock with the Connection close().
>
> Making the above change *seems* safe to me and (for what little it is
> worth) all the system tests continue to pass without issue on both the
> 0-9-1 and 0-10 paths.
>
> Can anyone see any issues with the above?
>
> -- Rob
>
> On 9 February 2015 at 12:15, Robbie Gemmell <ro...@gmail.com>
> wrote:
>
>> Hi Rob,
>>
>> Its a couple of years since I last did any real work on the 0-10
>> client, but around that time I did review some proposed changes
>> attempting removal of the MDL enough to know they likely introduced
>> new issues. The main thing I recall from back then is it being hard to
>> reason about the impact of such changes and the interaction of the
>> various locks due to the way the client threads can enter the
>> application space (e.g onMessage, exception listeners) and then
>> re-enter the client code while holding an unclear set of locks.
>>
>> What you wrote certainly makes logical sense in that if you always
>> acquire the MDL before the FM then even that behaviour shouldnt then
>> be an issue in terms of deadlocking between those two. The main
>> weirdness I can think comes of in terms of the stacktrace given in
>> that if the dispatcher is inside onMessage holding the MDL it could
>> then block the connection IOReceiver thread from completing whatever
>> it is attempting, leaving it unable to service new bytes arriving from
>> the broker (e.g should anything onMessage proceeds to do actually
>> generate new requests, or there being other sessions) as well as doing
>> anything else it would normally do, until that sessions onMessage
>> exists, which may itself cause different problems. Looking again at
>> the the given stacktrace, it does sound reasonable for the lock order
>> in connection close to be reversed, and I also wonder if the MDL is
>> truely needed when indicating the session closed due to an execution
>> exception. My hands are currently full on other things though so these
>> aren't areas I am going to be able to look at personally.
>>
>> Robbie
>>
>> On 7 February 2015 at 14:23, Rob Godfrey <ro...@gmail.com> wrote:
>> > Hi Rajith,
>> >
>> > so - I'm no expert on the locking model in the client... Logically I
>> would
>> > think that with the message delivery lock (MDL) being a session level
>> lock,
>> > and the failover mutex (FM) being connection wide, we should always
>> acquire
>> > the MDL before the FM.  Where we are currently acquiring the FM without
>> > acquiring the MDL I would (probably naively) think that we should first
>> be
>> > acquiring the MDL before the FM where the operation in question is
>> specific
>> > to a session (e.g. send a message, declare a queue, etc).  In the case
>> > where the operation is connection wide (which I think is pretty much
>> > limitted to connection closure) I would think that we would want to get
>> the
>> > MDLs for all of the sessions on the connection before getting the FM.
>> >
>> > Looking at the code I couldn't immediately see why such a change would
>> > cause an issue - if something is performing an operation against a
>> session
>> > in onMessage mode then it would currently hold the MDL... and if we are
>> > using receive() then holding the MDL is seemingly compatible with the
>> > definition of given by the javadoc.
>> >
>> > I expect I'm missing some subtlety here - can someone enlighten me?
>> >
>> > Thanks,
>> > Rob
>> >
>> > On 5 February 2015 at 15:16, Rajith Muditha Attapattu <
>> rajith77@gmail.com>
>> > wrote:
>> >
>> >> The consensus is, that there is no easy fix, other than rewriting some
>> key
>> >> pieces of the client.
>> >> Given that a new JMS client is being actively developed, we have decided
>> >> not to devote any significant time/resources on the older client.
>> >>
>> >> The new JMS client is AMQP 1.0 and the source is here
>> >> https://git-wip-us.apache.org/repos/asf?p=qpid-jms.git
>> >> Robbie is the best person to give an accurate picture of the status for
>> >> this project.
>> >>
>> >> Rajith
>> >>
>> >> On Thu, Feb 5, 2015 at 2:42 AM, Rob Godfrey <ro...@gmail.com>
>> >> wrote:
>> >>
>> >> > To answer the easy question first, 0.32 clients should be totally
>> >> > compatible with an 0.16 broker.
>> >> >
>> >> > In terms of the deadlock issue, I know there are a number of open
>> >> deadlock
>> >> > JIRAs that Robbie and Rajith have looked at in the past... Hopefully
>> one
>> >> of
>> >> > them will be able to chip in here and discuss the feasibility of
>> fixing
>> >> > your particular issue for 0.32.
>> >> >
>> >> > -- Rob
>> >> >
>> >> > On 4 February 2015 at 23:36, Helen Kwong <he...@gmail.com>
>> wrote:
>> >> >
>> >> > > Hi Qpid gurus,
>> >> > >
>> >> > > We are using 0.16 Java broker and client on 0-10, and we are running
>> >> into
>> >> > > deadlock issues on the client that involve AMQSession's
>> >> > > _messageDeliveryLock and AMQConnection's _failoverMutex, where
>> >> different
>> >> > > threads acquire them in different orders. This is leading to major
>> >> > > production headaches for us and has us very worried.
>> >> > >
>> >> > > (I've been looking at 0.16 code mostly but also skimmed the relevant
>> >> > parts
>> >> > > in 0.32, which seem largely the same in those places.)
>> >> > >
>> >> > > Deadlock Variety 1
>> >> > >
>> >> > > This is an example of a deadlock we see, where the IOReceiver thread
>> >> > > deadlocks with Session dispatcher thread (we have listeners that
>> call
>> >> > > session rollback or commit in onMessage()):
>> >> > >
>> >> > > IOReceiver
>> >> > > org.apache.qpid.client.AMQSession.closed(AMQSession.java:818)
>> >> > ---------->
>> >> > > waiting for session's messageDeliveryLock
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.client.AMQConnection.closeAllSessions(AMQConnection.java:938)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1282)
>> >> > >  ----------> acquires connection's failoverMutex
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.client.AMQSession_0_10.exception(AMQSession_0_10.java:907)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:182)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:32)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:55)
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:50)
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:32)
>> >> > > org.apache.qpid.transport.Method.delegate(Method.java:159)
>> >> > > org.apache.qpid.transport.Session.received(Session.java:585)
>> >> > > org.apache.qpid.transport.Connection.dispatch(Connection.java:412)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:64)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:40)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.transport.MethodDelegate.executionException(MethodDelegate.java:110)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:54)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:40)
>> >> > > org.apache.qpid.transport.Method.delegate(Method.java:159)
>> >> > > org.apache.qpid.transport.Connection.received(Connection.java:367)
>> >> > > org.apache.qpid.transport.Connection.received(Connection.java:65)
>> >> > > org.apache.qpid.transport.network.Assembler.emit(Assembler.java:97)
>> >> > >
>> >> org.apache.qpid.transport.network.Assembler.assemble(Assembler.java:198)
>> >> > >
>> org.apache.qpid.transport.network.Assembler.frame(Assembler.java:131)
>> >> > > org.apache.qpid.transport.network.Frame.delegate(Frame.java:128)
>> >> > >
>> >> org.apache.qpid.transport.network.Assembler.received(Assembler.java:102)
>> >> > >
>> org.apache.qpid.transport.network.Assembler.received(Assembler.java:44)
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.transport.network.InputHandler.next(InputHandler.java:189)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:105)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:44)
>> >> > >
>> >> org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:152)
>> >> > > java.lang.Thread.run(Thread.java:745)
>> >> > >
>> >> > > Session dispatcher thread
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1255)
>> >> > >  ---------> waiting for connection's failoverMutex
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
>> >> > >
>> org.apache.qpid.client.AMQSession_0_10.sync(AMQSession_0_10.java:1034)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.client.AMQSession_0_10.sendSuspendChannel(AMQSession_0_10.java:812)
>> >> > >
>> org.apache.qpid.client.AMQSession.suspendChannel(AMQSession.java:3075)
>> >> > > org.apache.qpid.client.AMQSession.rollback(AMQSession.java:1837)
>> >> > > common.messaging.QpidSession.rollback(QpidSession.java:211)
>> >> > >
>> >> > >
>> >> >
>> >>
>> common.messaging.QpidMessageHandler.rollbackSession(QpidMessageHandler.java:284)
>> >> > >
>> >> >
>> >>
>> common.messaging.QpidMessageHandler.onMessage(QpidMessageHandler.java:113)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:748)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:141)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:722)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:186)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:54)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.client.AMQSession$Dispatcher.notifyConsumer(AMQSession.java:3454)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.client.AMQSession$Dispatcher.dispatchMessage(AMQSession.java:3393)
>> >> > >   -----------> acquires session's messageDeliverylock
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.client.AMQSession$Dispatcher.access$1000(AMQSession.java:3180)
>> >> > > org.apache.qpid.client.AMQSession.dispatch(AMQSession.java:3173)
>> >> > >
>> >> > >
>> >> >
>> >>
>> org.apache.qpid.client.message.UnprocessedMessage.dispatch(UnprocessedMessage.java:54)
>> >> > >
>> org.apache.qpid.client.AMQSession$Dispatcher.run(AMQSession.java:3316)
>> >> > > java.lang.Thread.run(Thread.java:745)
>> >> > >
>> >> > > The problem is that the IOReceiver thread acquires failoverMutex
>> before
>> >> > > messageDeliveryLock (for each session), whereas the dispatcher
>> thread
>> >> > > acquires it in the other order. We also see potential problems where
>> >> > other
>> >> > > threads (instead of IOReceiver) can deadlock with the dispatcher
>> >> thread,
>> >> > as
>> >> > > long as it acquires failoverMutex before messageDeliveryLock.
>> Examples
>> >> we
>> >> > > can think of:
>> >> > >
>> >> > > A) Another thread calling AMQSession.close()
>> >> > > B) Another thread calling BasicMessageConsumer.close()
>> >> > > C) Same connection, different session's dispatcher thread, calling
>> >> > > rollback() or commit() -> sync() -> setCurrentException() ->
>> >> > > AMQConnection.exceptionReceived() ->
>> AMQConnection.closeAllSessions(),
>> >> > > which can try to acquire the messageDeliveryLock of another session
>> and
>> >> > > deadlock with the other session's dispatcher thread
>> >> > >
>> >> > >
>> >> > > Deadlock Variety 2:
>> >> > > From code inspection, it also appears that AMQConnection.close() can
>> >> > > deadlock with either AMQSession.close() or
>> BasicMessageConsumer.close()
>> >> > > (where the session / consumer is on the same connection). This is
>> >> because
>> >> > > AMQConnection.close() first acquires the messageDeliveryLock of all
>> its
>> >> > > sessions in the recursive doClose(), before trying to acquire the
>> >> > > connection's failoverMutex. But the Session / consumer's close()
>> >> acquires
>> >> > > the failoverMutex before messageDeliveryLock. We haven't seen this
>> >> happen
>> >> > > but would like to know if this is possible.
>> >> > >
>> >> > >
>> >> > > We'd really appreciate your help on this. Assuming these can be
>> fixed
>> >> in
>> >> > > 0.32, we are also wondering if clients are backward compatible --
>> i.e.,
>> >> > can
>> >> > > we upgrade only our client to 0.32 while continuing to use the 0.16
>> >> > broker?
>> >> > >
>> >> > > Thanks,
>> >> > > Helen
>> >> > >
>> >> >
>> >>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
>> For additional commands, e-mail: users-help@qpid.apache.org
>>
>>

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org


Re: Qpid Java client deadlocks

Posted by Rob Godfrey <ro...@gmail.com>.
Hi Robbie,

yes - my thinking is currently that the acquiring of the MDL in
AMQSession.closed() is incorrect.  The MDL seems to be there to provide JMS
semantics around the interaction between message delivery and other methods
on the (JMS) session object.  As such it should only really be acquired
from threads acting on behalf of the "application".  Application initiated
closes already hold the MDL by the time they get into AMQSession.closed()
so I think this can safely be removed.  In the other cases of the MDL being
acquired after the failover mutex (such as BasicMessageConsumer.close()) I
can see no reason why the lock acquisition order cannot be reversed (the
locks are pretty much taken out one after another) and by reversing them we
can prevent deadlock with the Connection close().

Making the above change *seems* safe to me and (for what little it is
worth) all the system tests continue to pass without issue on both the
0-9-1 and 0-10 paths.

Can anyone see any issues with the above?

-- Rob

On 9 February 2015 at 12:15, Robbie Gemmell <ro...@gmail.com>
wrote:

> Hi Rob,
>
> Its a couple of years since I last did any real work on the 0-10
> client, but around that time I did review some proposed changes
> attempting removal of the MDL enough to know they likely introduced
> new issues. The main thing I recall from back then is it being hard to
> reason about the impact of such changes and the interaction of the
> various locks due to the way the client threads can enter the
> application space (e.g onMessage, exception listeners) and then
> re-enter the client code while holding an unclear set of locks.
>
> What you wrote certainly makes logical sense in that if you always
> acquire the MDL before the FM then even that behaviour shouldnt then
> be an issue in terms of deadlocking between those two. The main
> weirdness I can think comes of in terms of the stacktrace given in
> that if the dispatcher is inside onMessage holding the MDL it could
> then block the connection IOReceiver thread from completing whatever
> it is attempting, leaving it unable to service new bytes arriving from
> the broker (e.g should anything onMessage proceeds to do actually
> generate new requests, or there being other sessions) as well as doing
> anything else it would normally do, until that sessions onMessage
> exists, which may itself cause different problems. Looking again at
> the the given stacktrace, it does sound reasonable for the lock order
> in connection close to be reversed, and I also wonder if the MDL is
> truely needed when indicating the session closed due to an execution
> exception. My hands are currently full on other things though so these
> aren't areas I am going to be able to look at personally.
>
> Robbie
>
> On 7 February 2015 at 14:23, Rob Godfrey <ro...@gmail.com> wrote:
> > Hi Rajith,
> >
> > so - I'm no expert on the locking model in the client... Logically I
> would
> > think that with the message delivery lock (MDL) being a session level
> lock,
> > and the failover mutex (FM) being connection wide, we should always
> acquire
> > the MDL before the FM.  Where we are currently acquiring the FM without
> > acquiring the MDL I would (probably naively) think that we should first
> be
> > acquiring the MDL before the FM where the operation in question is
> specific
> > to a session (e.g. send a message, declare a queue, etc).  In the case
> > where the operation is connection wide (which I think is pretty much
> > limitted to connection closure) I would think that we would want to get
> the
> > MDLs for all of the sessions on the connection before getting the FM.
> >
> > Looking at the code I couldn't immediately see why such a change would
> > cause an issue - if something is performing an operation against a
> session
> > in onMessage mode then it would currently hold the MDL... and if we are
> > using receive() then holding the MDL is seemingly compatible with the
> > definition of given by the javadoc.
> >
> > I expect I'm missing some subtlety here - can someone enlighten me?
> >
> > Thanks,
> > Rob
> >
> > On 5 February 2015 at 15:16, Rajith Muditha Attapattu <
> rajith77@gmail.com>
> > wrote:
> >
> >> The consensus is, that there is no easy fix, other than rewriting some
> key
> >> pieces of the client.
> >> Given that a new JMS client is being actively developed, we have decided
> >> not to devote any significant time/resources on the older client.
> >>
> >> The new JMS client is AMQP 1.0 and the source is here
> >> https://git-wip-us.apache.org/repos/asf?p=qpid-jms.git
> >> Robbie is the best person to give an accurate picture of the status for
> >> this project.
> >>
> >> Rajith
> >>
> >> On Thu, Feb 5, 2015 at 2:42 AM, Rob Godfrey <ro...@gmail.com>
> >> wrote:
> >>
> >> > To answer the easy question first, 0.32 clients should be totally
> >> > compatible with an 0.16 broker.
> >> >
> >> > In terms of the deadlock issue, I know there are a number of open
> >> deadlock
> >> > JIRAs that Robbie and Rajith have looked at in the past... Hopefully
> one
> >> of
> >> > them will be able to chip in here and discuss the feasibility of
> fixing
> >> > your particular issue for 0.32.
> >> >
> >> > -- Rob
> >> >
> >> > On 4 February 2015 at 23:36, Helen Kwong <he...@gmail.com>
> wrote:
> >> >
> >> > > Hi Qpid gurus,
> >> > >
> >> > > We are using 0.16 Java broker and client on 0-10, and we are running
> >> into
> >> > > deadlock issues on the client that involve AMQSession's
> >> > > _messageDeliveryLock and AMQConnection's _failoverMutex, where
> >> different
> >> > > threads acquire them in different orders. This is leading to major
> >> > > production headaches for us and has us very worried.
> >> > >
> >> > > (I've been looking at 0.16 code mostly but also skimmed the relevant
> >> > parts
> >> > > in 0.32, which seem largely the same in those places.)
> >> > >
> >> > > Deadlock Variety 1
> >> > >
> >> > > This is an example of a deadlock we see, where the IOReceiver thread
> >> > > deadlocks with Session dispatcher thread (we have listeners that
> call
> >> > > session rollback or commit in onMessage()):
> >> > >
> >> > > IOReceiver
> >> > > org.apache.qpid.client.AMQSession.closed(AMQSession.java:818)
> >> > ---------->
> >> > > waiting for session's messageDeliveryLock
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.client.AMQConnection.closeAllSessions(AMQConnection.java:938)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1282)
> >> > >  ----------> acquires connection's failoverMutex
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
> >> > >
> >> >
> >>
> org.apache.qpid.client.AMQSession_0_10.exception(AMQSession_0_10.java:907)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:182)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:32)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
> >> > >
> >> >
> >>
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:55)
> >> > >
> >> >
> >>
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:50)
> >> > >
> >> >
> >>
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:32)
> >> > > org.apache.qpid.transport.Method.delegate(Method.java:159)
> >> > > org.apache.qpid.transport.Session.received(Session.java:585)
> >> > > org.apache.qpid.transport.Connection.dispatch(Connection.java:412)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:64)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:40)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.transport.MethodDelegate.executionException(MethodDelegate.java:110)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:54)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:40)
> >> > > org.apache.qpid.transport.Method.delegate(Method.java:159)
> >> > > org.apache.qpid.transport.Connection.received(Connection.java:367)
> >> > > org.apache.qpid.transport.Connection.received(Connection.java:65)
> >> > > org.apache.qpid.transport.network.Assembler.emit(Assembler.java:97)
> >> > >
> >> org.apache.qpid.transport.network.Assembler.assemble(Assembler.java:198)
> >> > >
> org.apache.qpid.transport.network.Assembler.frame(Assembler.java:131)
> >> > > org.apache.qpid.transport.network.Frame.delegate(Frame.java:128)
> >> > >
> >> org.apache.qpid.transport.network.Assembler.received(Assembler.java:102)
> >> > >
> org.apache.qpid.transport.network.Assembler.received(Assembler.java:44)
> >> > >
> >> >
> >>
> org.apache.qpid.transport.network.InputHandler.next(InputHandler.java:189)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:105)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:44)
> >> > >
> >> org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:152)
> >> > > java.lang.Thread.run(Thread.java:745)
> >> > >
> >> > > Session dispatcher thread
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1255)
> >> > >  ---------> waiting for connection's failoverMutex
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
> >> > >
> org.apache.qpid.client.AMQSession_0_10.sync(AMQSession_0_10.java:1034)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.client.AMQSession_0_10.sendSuspendChannel(AMQSession_0_10.java:812)
> >> > >
> org.apache.qpid.client.AMQSession.suspendChannel(AMQSession.java:3075)
> >> > > org.apache.qpid.client.AMQSession.rollback(AMQSession.java:1837)
> >> > > common.messaging.QpidSession.rollback(QpidSession.java:211)
> >> > >
> >> > >
> >> >
> >>
> common.messaging.QpidMessageHandler.rollbackSession(QpidMessageHandler.java:284)
> >> > >
> >> >
> >>
> common.messaging.QpidMessageHandler.onMessage(QpidMessageHandler.java:113)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:748)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:141)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:722)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:186)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:54)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.client.AMQSession$Dispatcher.notifyConsumer(AMQSession.java:3454)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.client.AMQSession$Dispatcher.dispatchMessage(AMQSession.java:3393)
> >> > >   -----------> acquires session's messageDeliverylock
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.client.AMQSession$Dispatcher.access$1000(AMQSession.java:3180)
> >> > > org.apache.qpid.client.AMQSession.dispatch(AMQSession.java:3173)
> >> > >
> >> > >
> >> >
> >>
> org.apache.qpid.client.message.UnprocessedMessage.dispatch(UnprocessedMessage.java:54)
> >> > >
> org.apache.qpid.client.AMQSession$Dispatcher.run(AMQSession.java:3316)
> >> > > java.lang.Thread.run(Thread.java:745)
> >> > >
> >> > > The problem is that the IOReceiver thread acquires failoverMutex
> before
> >> > > messageDeliveryLock (for each session), whereas the dispatcher
> thread
> >> > > acquires it in the other order. We also see potential problems where
> >> > other
> >> > > threads (instead of IOReceiver) can deadlock with the dispatcher
> >> thread,
> >> > as
> >> > > long as it acquires failoverMutex before messageDeliveryLock.
> Examples
> >> we
> >> > > can think of:
> >> > >
> >> > > A) Another thread calling AMQSession.close()
> >> > > B) Another thread calling BasicMessageConsumer.close()
> >> > > C) Same connection, different session's dispatcher thread, calling
> >> > > rollback() or commit() -> sync() -> setCurrentException() ->
> >> > > AMQConnection.exceptionReceived() ->
> AMQConnection.closeAllSessions(),
> >> > > which can try to acquire the messageDeliveryLock of another session
> and
> >> > > deadlock with the other session's dispatcher thread
> >> > >
> >> > >
> >> > > Deadlock Variety 2:
> >> > > From code inspection, it also appears that AMQConnection.close() can
> >> > > deadlock with either AMQSession.close() or
> BasicMessageConsumer.close()
> >> > > (where the session / consumer is on the same connection). This is
> >> because
> >> > > AMQConnection.close() first acquires the messageDeliveryLock of all
> its
> >> > > sessions in the recursive doClose(), before trying to acquire the
> >> > > connection's failoverMutex. But the Session / consumer's close()
> >> acquires
> >> > > the failoverMutex before messageDeliveryLock. We haven't seen this
> >> happen
> >> > > but would like to know if this is possible.
> >> > >
> >> > >
> >> > > We'd really appreciate your help on this. Assuming these can be
> fixed
> >> in
> >> > > 0.32, we are also wondering if clients are backward compatible --
> i.e.,
> >> > can
> >> > > we upgrade only our client to 0.32 while continuing to use the 0.16
> >> > broker?
> >> > >
> >> > > Thanks,
> >> > > Helen
> >> > >
> >> >
> >>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
> For additional commands, e-mail: users-help@qpid.apache.org
>
>

Re: Qpid Java client deadlocks

Posted by Robbie Gemmell <ro...@gmail.com>.
Hi Rob,

Its a couple of years since I last did any real work on the 0-10
client, but around that time I did review some proposed changes
attempting removal of the MDL enough to know they likely introduced
new issues. The main thing I recall from back then is it being hard to
reason about the impact of such changes and the interaction of the
various locks due to the way the client threads can enter the
application space (e.g onMessage, exception listeners) and then
re-enter the client code while holding an unclear set of locks.

What you wrote certainly makes logical sense in that if you always
acquire the MDL before the FM then even that behaviour shouldnt then
be an issue in terms of deadlocking between those two. The main
weirdness I can think comes of in terms of the stacktrace given in
that if the dispatcher is inside onMessage holding the MDL it could
then block the connection IOReceiver thread from completing whatever
it is attempting, leaving it unable to service new bytes arriving from
the broker (e.g should anything onMessage proceeds to do actually
generate new requests, or there being other sessions) as well as doing
anything else it would normally do, until that sessions onMessage
exists, which may itself cause different problems. Looking again at
the the given stacktrace, it does sound reasonable for the lock order
in connection close to be reversed, and I also wonder if the MDL is
truely needed when indicating the session closed due to an execution
exception. My hands are currently full on other things though so these
aren't areas I am going to be able to look at personally.

Robbie

On 7 February 2015 at 14:23, Rob Godfrey <ro...@gmail.com> wrote:
> Hi Rajith,
>
> so - I'm no expert on the locking model in the client... Logically I would
> think that with the message delivery lock (MDL) being a session level lock,
> and the failover mutex (FM) being connection wide, we should always acquire
> the MDL before the FM.  Where we are currently acquiring the FM without
> acquiring the MDL I would (probably naively) think that we should first be
> acquiring the MDL before the FM where the operation in question is specific
> to a session (e.g. send a message, declare a queue, etc).  In the case
> where the operation is connection wide (which I think is pretty much
> limitted to connection closure) I would think that we would want to get the
> MDLs for all of the sessions on the connection before getting the FM.
>
> Looking at the code I couldn't immediately see why such a change would
> cause an issue - if something is performing an operation against a session
> in onMessage mode then it would currently hold the MDL... and if we are
> using receive() then holding the MDL is seemingly compatible with the
> definition of given by the javadoc.
>
> I expect I'm missing some subtlety here - can someone enlighten me?
>
> Thanks,
> Rob
>
> On 5 February 2015 at 15:16, Rajith Muditha Attapattu <ra...@gmail.com>
> wrote:
>
>> The consensus is, that there is no easy fix, other than rewriting some key
>> pieces of the client.
>> Given that a new JMS client is being actively developed, we have decided
>> not to devote any significant time/resources on the older client.
>>
>> The new JMS client is AMQP 1.0 and the source is here
>> https://git-wip-us.apache.org/repos/asf?p=qpid-jms.git
>> Robbie is the best person to give an accurate picture of the status for
>> this project.
>>
>> Rajith
>>
>> On Thu, Feb 5, 2015 at 2:42 AM, Rob Godfrey <ro...@gmail.com>
>> wrote:
>>
>> > To answer the easy question first, 0.32 clients should be totally
>> > compatible with an 0.16 broker.
>> >
>> > In terms of the deadlock issue, I know there are a number of open
>> deadlock
>> > JIRAs that Robbie and Rajith have looked at in the past... Hopefully one
>> of
>> > them will be able to chip in here and discuss the feasibility of fixing
>> > your particular issue for 0.32.
>> >
>> > -- Rob
>> >
>> > On 4 February 2015 at 23:36, Helen Kwong <he...@gmail.com> wrote:
>> >
>> > > Hi Qpid gurus,
>> > >
>> > > We are using 0.16 Java broker and client on 0-10, and we are running
>> into
>> > > deadlock issues on the client that involve AMQSession's
>> > > _messageDeliveryLock and AMQConnection's _failoverMutex, where
>> different
>> > > threads acquire them in different orders. This is leading to major
>> > > production headaches for us and has us very worried.
>> > >
>> > > (I've been looking at 0.16 code mostly but also skimmed the relevant
>> > parts
>> > > in 0.32, which seem largely the same in those places.)
>> > >
>> > > Deadlock Variety 1
>> > >
>> > > This is an example of a deadlock we see, where the IOReceiver thread
>> > > deadlocks with Session dispatcher thread (we have listeners that call
>> > > session rollback or commit in onMessage()):
>> > >
>> > > IOReceiver
>> > > org.apache.qpid.client.AMQSession.closed(AMQSession.java:818)
>> > ---------->
>> > > waiting for session's messageDeliveryLock
>> > >
>> > >
>> >
>> org.apache.qpid.client.AMQConnection.closeAllSessions(AMQConnection.java:938)
>> > >
>> > >
>> >
>> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1282)
>> > >  ----------> acquires connection's failoverMutex
>> > >
>> > >
>> >
>> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
>> > >
>> >
>> org.apache.qpid.client.AMQSession_0_10.exception(AMQSession_0_10.java:907)
>> > >
>> > >
>> >
>> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:182)
>> > >
>> > >
>> >
>> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:32)
>> > >
>> > >
>> >
>> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
>> > >
>> >
>> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:55)
>> > >
>> >
>> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:50)
>> > >
>> >
>> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:32)
>> > > org.apache.qpid.transport.Method.delegate(Method.java:159)
>> > > org.apache.qpid.transport.Session.received(Session.java:585)
>> > > org.apache.qpid.transport.Connection.dispatch(Connection.java:412)
>> > >
>> > >
>> >
>> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:64)
>> > >
>> > >
>> >
>> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:40)
>> > >
>> > >
>> >
>> org.apache.qpid.transport.MethodDelegate.executionException(MethodDelegate.java:110)
>> > >
>> > >
>> >
>> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
>> > >
>> > >
>> >
>> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:54)
>> > >
>> > >
>> >
>> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:40)
>> > > org.apache.qpid.transport.Method.delegate(Method.java:159)
>> > > org.apache.qpid.transport.Connection.received(Connection.java:367)
>> > > org.apache.qpid.transport.Connection.received(Connection.java:65)
>> > > org.apache.qpid.transport.network.Assembler.emit(Assembler.java:97)
>> > >
>> org.apache.qpid.transport.network.Assembler.assemble(Assembler.java:198)
>> > > org.apache.qpid.transport.network.Assembler.frame(Assembler.java:131)
>> > > org.apache.qpid.transport.network.Frame.delegate(Frame.java:128)
>> > >
>> org.apache.qpid.transport.network.Assembler.received(Assembler.java:102)
>> > > org.apache.qpid.transport.network.Assembler.received(Assembler.java:44)
>> > >
>> >
>> org.apache.qpid.transport.network.InputHandler.next(InputHandler.java:189)
>> > >
>> > >
>> >
>> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:105)
>> > >
>> > >
>> >
>> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:44)
>> > >
>> org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:152)
>> > > java.lang.Thread.run(Thread.java:745)
>> > >
>> > > Session dispatcher thread
>> > >
>> > >
>> >
>> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1255)
>> > >  ---------> waiting for connection's failoverMutex
>> > >
>> > >
>> >
>> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
>> > > org.apache.qpid.client.AMQSession_0_10.sync(AMQSession_0_10.java:1034)
>> > >
>> > >
>> >
>> org.apache.qpid.client.AMQSession_0_10.sendSuspendChannel(AMQSession_0_10.java:812)
>> > > org.apache.qpid.client.AMQSession.suspendChannel(AMQSession.java:3075)
>> > > org.apache.qpid.client.AMQSession.rollback(AMQSession.java:1837)
>> > > common.messaging.QpidSession.rollback(QpidSession.java:211)
>> > >
>> > >
>> >
>> common.messaging.QpidMessageHandler.rollbackSession(QpidMessageHandler.java:284)
>> > >
>> >
>> common.messaging.QpidMessageHandler.onMessage(QpidMessageHandler.java:113)
>> > >
>> > >
>> >
>> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:748)
>> > >
>> > >
>> >
>> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:141)
>> > >
>> > >
>> >
>> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:722)
>> > >
>> > >
>> >
>> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:186)
>> > >
>> > >
>> >
>> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:54)
>> > >
>> > >
>> >
>> org.apache.qpid.client.AMQSession$Dispatcher.notifyConsumer(AMQSession.java:3454)
>> > >
>> > >
>> >
>> org.apache.qpid.client.AMQSession$Dispatcher.dispatchMessage(AMQSession.java:3393)
>> > >   -----------> acquires session's messageDeliverylock
>> > >
>> > >
>> >
>> org.apache.qpid.client.AMQSession$Dispatcher.access$1000(AMQSession.java:3180)
>> > > org.apache.qpid.client.AMQSession.dispatch(AMQSession.java:3173)
>> > >
>> > >
>> >
>> org.apache.qpid.client.message.UnprocessedMessage.dispatch(UnprocessedMessage.java:54)
>> > > org.apache.qpid.client.AMQSession$Dispatcher.run(AMQSession.java:3316)
>> > > java.lang.Thread.run(Thread.java:745)
>> > >
>> > > The problem is that the IOReceiver thread acquires failoverMutex before
>> > > messageDeliveryLock (for each session), whereas the dispatcher thread
>> > > acquires it in the other order. We also see potential problems where
>> > other
>> > > threads (instead of IOReceiver) can deadlock with the dispatcher
>> thread,
>> > as
>> > > long as it acquires failoverMutex before messageDeliveryLock. Examples
>> we
>> > > can think of:
>> > >
>> > > A) Another thread calling AMQSession.close()
>> > > B) Another thread calling BasicMessageConsumer.close()
>> > > C) Same connection, different session's dispatcher thread, calling
>> > > rollback() or commit() -> sync() -> setCurrentException() ->
>> > > AMQConnection.exceptionReceived() -> AMQConnection.closeAllSessions(),
>> > > which can try to acquire the messageDeliveryLock of another session and
>> > > deadlock with the other session's dispatcher thread
>> > >
>> > >
>> > > Deadlock Variety 2:
>> > > From code inspection, it also appears that AMQConnection.close() can
>> > > deadlock with either AMQSession.close() or BasicMessageConsumer.close()
>> > > (where the session / consumer is on the same connection). This is
>> because
>> > > AMQConnection.close() first acquires the messageDeliveryLock of all its
>> > > sessions in the recursive doClose(), before trying to acquire the
>> > > connection's failoverMutex. But the Session / consumer's close()
>> acquires
>> > > the failoverMutex before messageDeliveryLock. We haven't seen this
>> happen
>> > > but would like to know if this is possible.
>> > >
>> > >
>> > > We'd really appreciate your help on this. Assuming these can be fixed
>> in
>> > > 0.32, we are also wondering if clients are backward compatible -- i.e.,
>> > can
>> > > we upgrade only our client to 0.32 while continuing to use the 0.16
>> > broker?
>> > >
>> > > Thanks,
>> > > Helen
>> > >
>> >
>>

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org


Re: Qpid Java client deadlocks

Posted by Rajith Muditha Attapattu <ra...@gmail.com>.
Rob,

It's been a bit more than a year since I've last looked at the code. So I
don't remember all the details, unless I look at it again.
I made a reasonable effort (and other folks too) to address these issues,
but ran into other issues that made the code unstable.
There are several variations of this deadlock and there are atleast 3
distinct types of deadlocks that exists in the code base (if memory serves
me right). This is a can of worms I'm not inclined to open up again.

Fixing these required major effort. Therefore the upstream (qpid community)
resources were directed to the new JMS client worked on by Robbie atm.

The code at github I pointed to was a private initiative on my part and
supported by my employer for a very specific need.
The code was not put upstream as the community was already working towards
a new AMQP 1.0 based JMS client.
Having yet another JMS client would create more confusion, nor did we have
the resources to actively support it.

Regards,

Rajith



On Sat, Feb 7, 2015 at 9:23 AM, Rob Godfrey <ro...@gmail.com> wrote:

> Hi Rajith,
>
> so - I'm no expert on the locking model in the client... Logically I would
> think that with the message delivery lock (MDL) being a session level lock,
> and the failover mutex (FM) being connection wide, we should always acquire
> the MDL before the FM.  Where we are currently acquiring the FM without
> acquiring the MDL I would (probably naively) think that we should first be
> acquiring the MDL before the FM where the operation in question is specific
> to a session (e.g. send a message, declare a queue, etc).  In the case
> where the operation is connection wide (which I think is pretty much
> limitted to connection closure) I would think that we would want to get the
> MDLs for all of the sessions on the connection before getting the FM.
>
> Looking at the code I couldn't immediately see why such a change would
> cause an issue - if something is performing an operation against a session
> in onMessage mode then it would currently hold the MDL... and if we are
> using receive() then holding the MDL is seemingly compatible with the
> definition of given by the javadoc.
>
> I expect I'm missing some subtlety here - can someone enlighten me?
>
> Thanks,
> Rob
>
> On 5 February 2015 at 15:16, Rajith Muditha Attapattu <ra...@gmail.com>
> wrote:
>
> > The consensus is, that there is no easy fix, other than rewriting some
> key
> > pieces of the client.
> > Given that a new JMS client is being actively developed, we have decided
> > not to devote any significant time/resources on the older client.
> >
> > The new JMS client is AMQP 1.0 and the source is here
> > https://git-wip-us.apache.org/repos/asf?p=qpid-jms.git
> > Robbie is the best person to give an accurate picture of the status for
> > this project.
> >
> > Rajith
> >
> > On Thu, Feb 5, 2015 at 2:42 AM, Rob Godfrey <ro...@gmail.com>
> > wrote:
> >
> > > To answer the easy question first, 0.32 clients should be totally
> > > compatible with an 0.16 broker.
> > >
> > > In terms of the deadlock issue, I know there are a number of open
> > deadlock
> > > JIRAs that Robbie and Rajith have looked at in the past... Hopefully
> one
> > of
> > > them will be able to chip in here and discuss the feasibility of fixing
> > > your particular issue for 0.32.
> > >
> > > -- Rob
> > >
> > > On 4 February 2015 at 23:36, Helen Kwong <he...@gmail.com> wrote:
> > >
> > > > Hi Qpid gurus,
> > > >
> > > > We are using 0.16 Java broker and client on 0-10, and we are running
> > into
> > > > deadlock issues on the client that involve AMQSession's
> > > > _messageDeliveryLock and AMQConnection's _failoverMutex, where
> > different
> > > > threads acquire them in different orders. This is leading to major
> > > > production headaches for us and has us very worried.
> > > >
> > > > (I've been looking at 0.16 code mostly but also skimmed the relevant
> > > parts
> > > > in 0.32, which seem largely the same in those places.)
> > > >
> > > > Deadlock Variety 1
> > > >
> > > > This is an example of a deadlock we see, where the IOReceiver thread
> > > > deadlocks with Session dispatcher thread (we have listeners that call
> > > > session rollback or commit in onMessage()):
> > > >
> > > > IOReceiver
> > > > org.apache.qpid.client.AMQSession.closed(AMQSession.java:818)
> > > ---------->
> > > > waiting for session's messageDeliveryLock
> > > >
> > > >
> > >
> >
> org.apache.qpid.client.AMQConnection.closeAllSessions(AMQConnection.java:938)
> > > >
> > > >
> > >
> >
> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1282)
> > > >  ----------> acquires connection's failoverMutex
> > > >
> > > >
> > >
> >
> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
> > > >
> > >
> >
> org.apache.qpid.client.AMQSession_0_10.exception(AMQSession_0_10.java:907)
> > > >
> > > >
> > >
> >
> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:182)
> > > >
> > > >
> > >
> >
> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:32)
> > > >
> > > >
> > >
> >
> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
> > > >
> > >
> >
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:55)
> > > >
> > >
> >
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:50)
> > > >
> > >
> >
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:32)
> > > > org.apache.qpid.transport.Method.delegate(Method.java:159)
> > > > org.apache.qpid.transport.Session.received(Session.java:585)
> > > > org.apache.qpid.transport.Connection.dispatch(Connection.java:412)
> > > >
> > > >
> > >
> >
> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:64)
> > > >
> > > >
> > >
> >
> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:40)
> > > >
> > > >
> > >
> >
> org.apache.qpid.transport.MethodDelegate.executionException(MethodDelegate.java:110)
> > > >
> > > >
> > >
> >
> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
> > > >
> > > >
> > >
> >
> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:54)
> > > >
> > > >
> > >
> >
> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:40)
> > > > org.apache.qpid.transport.Method.delegate(Method.java:159)
> > > > org.apache.qpid.transport.Connection.received(Connection.java:367)
> > > > org.apache.qpid.transport.Connection.received(Connection.java:65)
> > > > org.apache.qpid.transport.network.Assembler.emit(Assembler.java:97)
> > > >
> > org.apache.qpid.transport.network.Assembler.assemble(Assembler.java:198)
> > > > org.apache.qpid.transport.network.Assembler.frame(Assembler.java:131)
> > > > org.apache.qpid.transport.network.Frame.delegate(Frame.java:128)
> > > >
> > org.apache.qpid.transport.network.Assembler.received(Assembler.java:102)
> > > >
> org.apache.qpid.transport.network.Assembler.received(Assembler.java:44)
> > > >
> > >
> >
> org.apache.qpid.transport.network.InputHandler.next(InputHandler.java:189)
> > > >
> > > >
> > >
> >
> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:105)
> > > >
> > > >
> > >
> >
> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:44)
> > > >
> > org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:152)
> > > > java.lang.Thread.run(Thread.java:745)
> > > >
> > > > Session dispatcher thread
> > > >
> > > >
> > >
> >
> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1255)
> > > >  ---------> waiting for connection's failoverMutex
> > > >
> > > >
> > >
> >
> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
> > > >
> org.apache.qpid.client.AMQSession_0_10.sync(AMQSession_0_10.java:1034)
> > > >
> > > >
> > >
> >
> org.apache.qpid.client.AMQSession_0_10.sendSuspendChannel(AMQSession_0_10.java:812)
> > > >
> org.apache.qpid.client.AMQSession.suspendChannel(AMQSession.java:3075)
> > > > org.apache.qpid.client.AMQSession.rollback(AMQSession.java:1837)
> > > > common.messaging.QpidSession.rollback(QpidSession.java:211)
> > > >
> > > >
> > >
> >
> common.messaging.QpidMessageHandler.rollbackSession(QpidMessageHandler.java:284)
> > > >
> > >
> >
> common.messaging.QpidMessageHandler.onMessage(QpidMessageHandler.java:113)
> > > >
> > > >
> > >
> >
> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:748)
> > > >
> > > >
> > >
> >
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:141)
> > > >
> > > >
> > >
> >
> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:722)
> > > >
> > > >
> > >
> >
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:186)
> > > >
> > > >
> > >
> >
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:54)
> > > >
> > > >
> > >
> >
> org.apache.qpid.client.AMQSession$Dispatcher.notifyConsumer(AMQSession.java:3454)
> > > >
> > > >
> > >
> >
> org.apache.qpid.client.AMQSession$Dispatcher.dispatchMessage(AMQSession.java:3393)
> > > >   -----------> acquires session's messageDeliverylock
> > > >
> > > >
> > >
> >
> org.apache.qpid.client.AMQSession$Dispatcher.access$1000(AMQSession.java:3180)
> > > > org.apache.qpid.client.AMQSession.dispatch(AMQSession.java:3173)
> > > >
> > > >
> > >
> >
> org.apache.qpid.client.message.UnprocessedMessage.dispatch(UnprocessedMessage.java:54)
> > > >
> org.apache.qpid.client.AMQSession$Dispatcher.run(AMQSession.java:3316)
> > > > java.lang.Thread.run(Thread.java:745)
> > > >
> > > > The problem is that the IOReceiver thread acquires failoverMutex
> before
> > > > messageDeliveryLock (for each session), whereas the dispatcher thread
> > > > acquires it in the other order. We also see potential problems where
> > > other
> > > > threads (instead of IOReceiver) can deadlock with the dispatcher
> > thread,
> > > as
> > > > long as it acquires failoverMutex before messageDeliveryLock.
> Examples
> > we
> > > > can think of:
> > > >
> > > > A) Another thread calling AMQSession.close()
> > > > B) Another thread calling BasicMessageConsumer.close()
> > > > C) Same connection, different session's dispatcher thread, calling
> > > > rollback() or commit() -> sync() -> setCurrentException() ->
> > > > AMQConnection.exceptionReceived() ->
> AMQConnection.closeAllSessions(),
> > > > which can try to acquire the messageDeliveryLock of another session
> and
> > > > deadlock with the other session's dispatcher thread
> > > >
> > > >
> > > > Deadlock Variety 2:
> > > > From code inspection, it also appears that AMQConnection.close() can
> > > > deadlock with either AMQSession.close() or
> BasicMessageConsumer.close()
> > > > (where the session / consumer is on the same connection). This is
> > because
> > > > AMQConnection.close() first acquires the messageDeliveryLock of all
> its
> > > > sessions in the recursive doClose(), before trying to acquire the
> > > > connection's failoverMutex. But the Session / consumer's close()
> > acquires
> > > > the failoverMutex before messageDeliveryLock. We haven't seen this
> > happen
> > > > but would like to know if this is possible.
> > > >
> > > >
> > > > We'd really appreciate your help on this. Assuming these can be fixed
> > in
> > > > 0.32, we are also wondering if clients are backward compatible --
> i.e.,
> > > can
> > > > we upgrade only our client to 0.32 while continuing to use the 0.16
> > > broker?
> > > >
> > > > Thanks,
> > > > Helen
> > > >
> > >
> >
>

Re: Qpid Java client deadlocks

Posted by Rob Godfrey <ro...@gmail.com>.
Hi Rajith,

so - I'm no expert on the locking model in the client... Logically I would
think that with the message delivery lock (MDL) being a session level lock,
and the failover mutex (FM) being connection wide, we should always acquire
the MDL before the FM.  Where we are currently acquiring the FM without
acquiring the MDL I would (probably naively) think that we should first be
acquiring the MDL before the FM where the operation in question is specific
to a session (e.g. send a message, declare a queue, etc).  In the case
where the operation is connection wide (which I think is pretty much
limitted to connection closure) I would think that we would want to get the
MDLs for all of the sessions on the connection before getting the FM.

Looking at the code I couldn't immediately see why such a change would
cause an issue - if something is performing an operation against a session
in onMessage mode then it would currently hold the MDL... and if we are
using receive() then holding the MDL is seemingly compatible with the
definition of given by the javadoc.

I expect I'm missing some subtlety here - can someone enlighten me?

Thanks,
Rob

On 5 February 2015 at 15:16, Rajith Muditha Attapattu <ra...@gmail.com>
wrote:

> The consensus is, that there is no easy fix, other than rewriting some key
> pieces of the client.
> Given that a new JMS client is being actively developed, we have decided
> not to devote any significant time/resources on the older client.
>
> The new JMS client is AMQP 1.0 and the source is here
> https://git-wip-us.apache.org/repos/asf?p=qpid-jms.git
> Robbie is the best person to give an accurate picture of the status for
> this project.
>
> Rajith
>
> On Thu, Feb 5, 2015 at 2:42 AM, Rob Godfrey <ro...@gmail.com>
> wrote:
>
> > To answer the easy question first, 0.32 clients should be totally
> > compatible with an 0.16 broker.
> >
> > In terms of the deadlock issue, I know there are a number of open
> deadlock
> > JIRAs that Robbie and Rajith have looked at in the past... Hopefully one
> of
> > them will be able to chip in here and discuss the feasibility of fixing
> > your particular issue for 0.32.
> >
> > -- Rob
> >
> > On 4 February 2015 at 23:36, Helen Kwong <he...@gmail.com> wrote:
> >
> > > Hi Qpid gurus,
> > >
> > > We are using 0.16 Java broker and client on 0-10, and we are running
> into
> > > deadlock issues on the client that involve AMQSession's
> > > _messageDeliveryLock and AMQConnection's _failoverMutex, where
> different
> > > threads acquire them in different orders. This is leading to major
> > > production headaches for us and has us very worried.
> > >
> > > (I've been looking at 0.16 code mostly but also skimmed the relevant
> > parts
> > > in 0.32, which seem largely the same in those places.)
> > >
> > > Deadlock Variety 1
> > >
> > > This is an example of a deadlock we see, where the IOReceiver thread
> > > deadlocks with Session dispatcher thread (we have listeners that call
> > > session rollback or commit in onMessage()):
> > >
> > > IOReceiver
> > > org.apache.qpid.client.AMQSession.closed(AMQSession.java:818)
> > ---------->
> > > waiting for session's messageDeliveryLock
> > >
> > >
> >
> org.apache.qpid.client.AMQConnection.closeAllSessions(AMQConnection.java:938)
> > >
> > >
> >
> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1282)
> > >  ----------> acquires connection's failoverMutex
> > >
> > >
> >
> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
> > >
> >
> org.apache.qpid.client.AMQSession_0_10.exception(AMQSession_0_10.java:907)
> > >
> > >
> >
> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:182)
> > >
> > >
> >
> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:32)
> > >
> > >
> >
> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
> > >
> >
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:55)
> > >
> >
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:50)
> > >
> >
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:32)
> > > org.apache.qpid.transport.Method.delegate(Method.java:159)
> > > org.apache.qpid.transport.Session.received(Session.java:585)
> > > org.apache.qpid.transport.Connection.dispatch(Connection.java:412)
> > >
> > >
> >
> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:64)
> > >
> > >
> >
> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:40)
> > >
> > >
> >
> org.apache.qpid.transport.MethodDelegate.executionException(MethodDelegate.java:110)
> > >
> > >
> >
> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
> > >
> > >
> >
> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:54)
> > >
> > >
> >
> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:40)
> > > org.apache.qpid.transport.Method.delegate(Method.java:159)
> > > org.apache.qpid.transport.Connection.received(Connection.java:367)
> > > org.apache.qpid.transport.Connection.received(Connection.java:65)
> > > org.apache.qpid.transport.network.Assembler.emit(Assembler.java:97)
> > >
> org.apache.qpid.transport.network.Assembler.assemble(Assembler.java:198)
> > > org.apache.qpid.transport.network.Assembler.frame(Assembler.java:131)
> > > org.apache.qpid.transport.network.Frame.delegate(Frame.java:128)
> > >
> org.apache.qpid.transport.network.Assembler.received(Assembler.java:102)
> > > org.apache.qpid.transport.network.Assembler.received(Assembler.java:44)
> > >
> >
> org.apache.qpid.transport.network.InputHandler.next(InputHandler.java:189)
> > >
> > >
> >
> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:105)
> > >
> > >
> >
> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:44)
> > >
> org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:152)
> > > java.lang.Thread.run(Thread.java:745)
> > >
> > > Session dispatcher thread
> > >
> > >
> >
> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1255)
> > >  ---------> waiting for connection's failoverMutex
> > >
> > >
> >
> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
> > > org.apache.qpid.client.AMQSession_0_10.sync(AMQSession_0_10.java:1034)
> > >
> > >
> >
> org.apache.qpid.client.AMQSession_0_10.sendSuspendChannel(AMQSession_0_10.java:812)
> > > org.apache.qpid.client.AMQSession.suspendChannel(AMQSession.java:3075)
> > > org.apache.qpid.client.AMQSession.rollback(AMQSession.java:1837)
> > > common.messaging.QpidSession.rollback(QpidSession.java:211)
> > >
> > >
> >
> common.messaging.QpidMessageHandler.rollbackSession(QpidMessageHandler.java:284)
> > >
> >
> common.messaging.QpidMessageHandler.onMessage(QpidMessageHandler.java:113)
> > >
> > >
> >
> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:748)
> > >
> > >
> >
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:141)
> > >
> > >
> >
> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:722)
> > >
> > >
> >
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:186)
> > >
> > >
> >
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:54)
> > >
> > >
> >
> org.apache.qpid.client.AMQSession$Dispatcher.notifyConsumer(AMQSession.java:3454)
> > >
> > >
> >
> org.apache.qpid.client.AMQSession$Dispatcher.dispatchMessage(AMQSession.java:3393)
> > >   -----------> acquires session's messageDeliverylock
> > >
> > >
> >
> org.apache.qpid.client.AMQSession$Dispatcher.access$1000(AMQSession.java:3180)
> > > org.apache.qpid.client.AMQSession.dispatch(AMQSession.java:3173)
> > >
> > >
> >
> org.apache.qpid.client.message.UnprocessedMessage.dispatch(UnprocessedMessage.java:54)
> > > org.apache.qpid.client.AMQSession$Dispatcher.run(AMQSession.java:3316)
> > > java.lang.Thread.run(Thread.java:745)
> > >
> > > The problem is that the IOReceiver thread acquires failoverMutex before
> > > messageDeliveryLock (for each session), whereas the dispatcher thread
> > > acquires it in the other order. We also see potential problems where
> > other
> > > threads (instead of IOReceiver) can deadlock with the dispatcher
> thread,
> > as
> > > long as it acquires failoverMutex before messageDeliveryLock. Examples
> we
> > > can think of:
> > >
> > > A) Another thread calling AMQSession.close()
> > > B) Another thread calling BasicMessageConsumer.close()
> > > C) Same connection, different session's dispatcher thread, calling
> > > rollback() or commit() -> sync() -> setCurrentException() ->
> > > AMQConnection.exceptionReceived() -> AMQConnection.closeAllSessions(),
> > > which can try to acquire the messageDeliveryLock of another session and
> > > deadlock with the other session's dispatcher thread
> > >
> > >
> > > Deadlock Variety 2:
> > > From code inspection, it also appears that AMQConnection.close() can
> > > deadlock with either AMQSession.close() or BasicMessageConsumer.close()
> > > (where the session / consumer is on the same connection). This is
> because
> > > AMQConnection.close() first acquires the messageDeliveryLock of all its
> > > sessions in the recursive doClose(), before trying to acquire the
> > > connection's failoverMutex. But the Session / consumer's close()
> acquires
> > > the failoverMutex before messageDeliveryLock. We haven't seen this
> happen
> > > but would like to know if this is possible.
> > >
> > >
> > > We'd really appreciate your help on this. Assuming these can be fixed
> in
> > > 0.32, we are also wondering if clients are backward compatible -- i.e.,
> > can
> > > we upgrade only our client to 0.32 while continuing to use the 0.16
> > broker?
> > >
> > > Thanks,
> > > Helen
> > >
> >
>

Re: Qpid Java client deadlocks

Posted by Rajith Muditha Attapattu <ra...@gmail.com>.
John,

If you are unable to wait for the AMQP 1.0 JMS client to be completed, may
I suggest an interim solution that *may* work for you.

This solution was provided for a customer who was completely blocked due to
the above mentioned deadlocks.
It's not a Qpid supported client. But it may provide relief for your
situation until you are able to move to the new AMQP 1.0 based client.
Please make sure you test adequately to see if it works for your situation.

You could build it from the following tag.
https://github.com/rajith77/qpid/tree/dt_aug_21_2013/qpid/java

Please note the following features are not implemented in this client
- XA support
- Queue browser
- Foreign message support.

If you decide to give it a try, please email me at rajith@redhat.com and I
can provide the details on how to get it going.

Regards,

Rajith

On Thu, Feb 5, 2015 at 9:30 AM, John Buisson <jb...@salesforce.com>
wrote:

> We will definitely need a timeline on this.  We are dequeueing messages on
> 12 app servers per "pod" with 32 threads per server dequeuing messages.  We
> have over 65 pods.  This means, at any given time, we have around 25,000
> dequeue threads running.  While there are isolated systems, we are
> encountering deadlocks multiple times a day on multiple pods with most of
> our traffic not running on QPID yet.  We are stuck moving forward until
> this is resolved, as support has to reboot the brokers/app servers when
> this happens.
>
> Wanted to make sure you all realized this isn't a minor annoyance.  We are
> using QPID quite a lot and this is a blocker for us moving forward.
>
> On Thu, Feb 5, 2015 at 9:16 AM, Rajith Muditha Attapattu <
> rajith77@gmail.com> wrote:
>
>> The consensus is, that there is no easy fix, other than rewriting some
>> key pieces of the client.
>> Given that a new JMS client is being actively developed, we have decided
>> not to devote any significant time/resources on the older client.
>>
>> The new JMS client is AMQP 1.0 and the source is here
>> https://git-wip-us.apache.org/repos/asf?p=qpid-jms.git
>> Robbie is the best person to give an accurate picture of the status for
>> this project.
>>
>> Rajith
>>
>> On Thu, Feb 5, 2015 at 2:42 AM, Rob Godfrey <ro...@gmail.com>
>> wrote:
>>
>>> To answer the easy question first, 0.32 clients should be totally
>>> compatible with an 0.16 broker.
>>>
>>> In terms of the deadlock issue, I know there are a number of open
>>> deadlock
>>> JIRAs that Robbie and Rajith have looked at in the past... Hopefully one
>>> of
>>> them will be able to chip in here and discuss the feasibility of fixing
>>> your particular issue for 0.32.
>>>
>>> -- Rob
>>>
>>> On 4 February 2015 at 23:36, Helen Kwong <he...@gmail.com> wrote:
>>>
>>> > Hi Qpid gurus,
>>> >
>>> > We are using 0.16 Java broker and client on 0-10, and we are running
>>> into
>>> > deadlock issues on the client that involve AMQSession's
>>> > _messageDeliveryLock and AMQConnection's _failoverMutex, where
>>> different
>>> > threads acquire them in different orders. This is leading to major
>>> > production headaches for us and has us very worried.
>>> >
>>> > (I've been looking at 0.16 code mostly but also skimmed the relevant
>>> parts
>>> > in 0.32, which seem largely the same in those places.)
>>> >
>>> > Deadlock Variety 1
>>> >
>>> > This is an example of a deadlock we see, where the IOReceiver thread
>>> > deadlocks with Session dispatcher thread (we have listeners that call
>>> > session rollback or commit in onMessage()):
>>> >
>>> > IOReceiver
>>> > org.apache.qpid.client.AMQSession.closed(AMQSession.java:818)
>>> ---------->
>>> > waiting for session's messageDeliveryLock
>>> >
>>> >
>>> org.apache.qpid.client.AMQConnection.closeAllSessions(AMQConnection.java:938)
>>> >
>>> >
>>> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1282)
>>> >  ----------> acquires connection's failoverMutex
>>> >
>>> >
>>> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
>>> >
>>> org.apache.qpid.client.AMQSession_0_10.exception(AMQSession_0_10.java:907)
>>> >
>>> >
>>> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:182)
>>> >
>>> >
>>> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:32)
>>> >
>>> >
>>> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
>>> >
>>> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:55)
>>> >
>>> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:50)
>>> >
>>> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:32)
>>> > org.apache.qpid.transport.Method.delegate(Method.java:159)
>>> > org.apache.qpid.transport.Session.received(Session.java:585)
>>> > org.apache.qpid.transport.Connection.dispatch(Connection.java:412)
>>> >
>>> >
>>> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:64)
>>> >
>>> >
>>> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:40)
>>> >
>>> >
>>> org.apache.qpid.transport.MethodDelegate.executionException(MethodDelegate.java:110)
>>> >
>>> >
>>> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
>>> >
>>> >
>>> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:54)
>>> >
>>> >
>>> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:40)
>>> > org.apache.qpid.transport.Method.delegate(Method.java:159)
>>> > org.apache.qpid.transport.Connection.received(Connection.java:367)
>>> > org.apache.qpid.transport.Connection.received(Connection.java:65)
>>> > org.apache.qpid.transport.network.Assembler.emit(Assembler.java:97)
>>> >
>>> org.apache.qpid.transport.network.Assembler.assemble(Assembler.java:198)
>>> > org.apache.qpid.transport.network.Assembler.frame(Assembler.java:131)
>>> > org.apache.qpid.transport.network.Frame.delegate(Frame.java:128)
>>> >
>>> org.apache.qpid.transport.network.Assembler.received(Assembler.java:102)
>>> > org.apache.qpid.transport.network.Assembler.received(Assembler.java:44)
>>> >
>>> org.apache.qpid.transport.network.InputHandler.next(InputHandler.java:189)
>>> >
>>> >
>>> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:105)
>>> >
>>> >
>>> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:44)
>>> >
>>> org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:152)
>>> > java.lang.Thread.run(Thread.java:745)
>>> >
>>> > Session dispatcher thread
>>> >
>>> >
>>> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1255)
>>> >  ---------> waiting for connection's failoverMutex
>>> >
>>> >
>>> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
>>> > org.apache.qpid.client.AMQSession_0_10.sync(AMQSession_0_10.java:1034)
>>> >
>>> >
>>> org.apache.qpid.client.AMQSession_0_10.sendSuspendChannel(AMQSession_0_10.java:812)
>>> > org.apache.qpid.client.AMQSession.suspendChannel(AMQSession.java:3075)
>>> > org.apache.qpid.client.AMQSession.rollback(AMQSession.java:1837)
>>> > common.messaging.QpidSession.rollback(QpidSession.java:211)
>>> >
>>> >
>>> common.messaging.QpidMessageHandler.rollbackSession(QpidMessageHandler.java:284)
>>> >
>>> common.messaging.QpidMessageHandler.onMessage(QpidMessageHandler.java:113)
>>> >
>>> >
>>> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:748)
>>> >
>>> >
>>> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:141)
>>> >
>>> >
>>> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:722)
>>> >
>>> >
>>> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:186)
>>> >
>>> >
>>> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:54)
>>> >
>>> >
>>> org.apache.qpid.client.AMQSession$Dispatcher.notifyConsumer(AMQSession.java:3454)
>>> >
>>> >
>>> org.apache.qpid.client.AMQSession$Dispatcher.dispatchMessage(AMQSession.java:3393)
>>> >   -----------> acquires session's messageDeliverylock
>>> >
>>> >
>>> org.apache.qpid.client.AMQSession$Dispatcher.access$1000(AMQSession.java:3180)
>>> > org.apache.qpid.client.AMQSession.dispatch(AMQSession.java:3173)
>>> >
>>> >
>>> org.apache.qpid.client.message.UnprocessedMessage.dispatch(UnprocessedMessage.java:54)
>>> > org.apache.qpid.client.AMQSession$Dispatcher.run(AMQSession.java:3316)
>>> > java.lang.Thread.run(Thread.java:745)
>>> >
>>> > The problem is that the IOReceiver thread acquires failoverMutex before
>>> > messageDeliveryLock (for each session), whereas the dispatcher thread
>>> > acquires it in the other order. We also see potential problems where
>>> other
>>> > threads (instead of IOReceiver) can deadlock with the dispatcher
>>> thread, as
>>> > long as it acquires failoverMutex before messageDeliveryLock. Examples
>>> we
>>> > can think of:
>>> >
>>> > A) Another thread calling AMQSession.close()
>>> > B) Another thread calling BasicMessageConsumer.close()
>>> > C) Same connection, different session's dispatcher thread, calling
>>> > rollback() or commit() -> sync() -> setCurrentException() ->
>>> > AMQConnection.exceptionReceived() -> AMQConnection.closeAllSessions(),
>>> > which can try to acquire the messageDeliveryLock of another session and
>>> > deadlock with the other session's dispatcher thread
>>> >
>>> >
>>> > Deadlock Variety 2:
>>> > From code inspection, it also appears that AMQConnection.close() can
>>> > deadlock with either AMQSession.close() or BasicMessageConsumer.close()
>>> > (where the session / consumer is on the same connection). This is
>>> because
>>> > AMQConnection.close() first acquires the messageDeliveryLock of all its
>>> > sessions in the recursive doClose(), before trying to acquire the
>>> > connection's failoverMutex. But the Session / consumer's close()
>>> acquires
>>> > the failoverMutex before messageDeliveryLock. We haven't seen this
>>> happen
>>> > but would like to know if this is possible.
>>> >
>>> >
>>> > We'd really appreciate your help on this. Assuming these can be fixed
>>> in
>>> > 0.32, we are also wondering if clients are backward compatible --
>>> i.e., can
>>> > we upgrade only our client to 0.32 while continuing to use the 0.16
>>> broker?
>>> >
>>> > Thanks,
>>> > Helen
>>> >
>>>
>>
>>
>

Re: Qpid Java client deadlocks

Posted by John Buisson <jb...@salesforce.com>.
We will definitely need a timeline on this.  We are dequeueing messages on
12 app servers per "pod" with 32 threads per server dequeuing messages.  We
have over 65 pods.  This means, at any given time, we have around 25,000
dequeue threads running.  While there are isolated systems, we are
encountering deadlocks multiple times a day on multiple pods with most of
our traffic not running on QPID yet.  We are stuck moving forward until
this is resolved, as support has to reboot the brokers/app servers when
this happens.

Wanted to make sure you all realized this isn't a minor annoyance.  We are
using QPID quite a lot and this is a blocker for us moving forward.

On Thu, Feb 5, 2015 at 9:16 AM, Rajith Muditha Attapattu <rajith77@gmail.com
> wrote:

> The consensus is, that there is no easy fix, other than rewriting some key
> pieces of the client.
> Given that a new JMS client is being actively developed, we have decided
> not to devote any significant time/resources on the older client.
>
> The new JMS client is AMQP 1.0 and the source is here
> https://git-wip-us.apache.org/repos/asf?p=qpid-jms.git
> Robbie is the best person to give an accurate picture of the status for
> this project.
>
> Rajith
>
> On Thu, Feb 5, 2015 at 2:42 AM, Rob Godfrey <ro...@gmail.com>
> wrote:
>
>> To answer the easy question first, 0.32 clients should be totally
>> compatible with an 0.16 broker.
>>
>> In terms of the deadlock issue, I know there are a number of open deadlock
>> JIRAs that Robbie and Rajith have looked at in the past... Hopefully one
>> of
>> them will be able to chip in here and discuss the feasibility of fixing
>> your particular issue for 0.32.
>>
>> -- Rob
>>
>> On 4 February 2015 at 23:36, Helen Kwong <he...@gmail.com> wrote:
>>
>> > Hi Qpid gurus,
>> >
>> > We are using 0.16 Java broker and client on 0-10, and we are running
>> into
>> > deadlock issues on the client that involve AMQSession's
>> > _messageDeliveryLock and AMQConnection's _failoverMutex, where different
>> > threads acquire them in different orders. This is leading to major
>> > production headaches for us and has us very worried.
>> >
>> > (I've been looking at 0.16 code mostly but also skimmed the relevant
>> parts
>> > in 0.32, which seem largely the same in those places.)
>> >
>> > Deadlock Variety 1
>> >
>> > This is an example of a deadlock we see, where the IOReceiver thread
>> > deadlocks with Session dispatcher thread (we have listeners that call
>> > session rollback or commit in onMessage()):
>> >
>> > IOReceiver
>> > org.apache.qpid.client.AMQSession.closed(AMQSession.java:818)
>> ---------->
>> > waiting for session's messageDeliveryLock
>> >
>> >
>> org.apache.qpid.client.AMQConnection.closeAllSessions(AMQConnection.java:938)
>> >
>> >
>> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1282)
>> >  ----------> acquires connection's failoverMutex
>> >
>> >
>> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
>> >
>> org.apache.qpid.client.AMQSession_0_10.exception(AMQSession_0_10.java:907)
>> >
>> >
>> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:182)
>> >
>> >
>> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:32)
>> >
>> >
>> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
>> >
>> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:55)
>> >
>> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:50)
>> >
>> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:32)
>> > org.apache.qpid.transport.Method.delegate(Method.java:159)
>> > org.apache.qpid.transport.Session.received(Session.java:585)
>> > org.apache.qpid.transport.Connection.dispatch(Connection.java:412)
>> >
>> >
>> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:64)
>> >
>> >
>> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:40)
>> >
>> >
>> org.apache.qpid.transport.MethodDelegate.executionException(MethodDelegate.java:110)
>> >
>> >
>> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
>> >
>> >
>> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:54)
>> >
>> >
>> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:40)
>> > org.apache.qpid.transport.Method.delegate(Method.java:159)
>> > org.apache.qpid.transport.Connection.received(Connection.java:367)
>> > org.apache.qpid.transport.Connection.received(Connection.java:65)
>> > org.apache.qpid.transport.network.Assembler.emit(Assembler.java:97)
>> > org.apache.qpid.transport.network.Assembler.assemble(Assembler.java:198)
>> > org.apache.qpid.transport.network.Assembler.frame(Assembler.java:131)
>> > org.apache.qpid.transport.network.Frame.delegate(Frame.java:128)
>> > org.apache.qpid.transport.network.Assembler.received(Assembler.java:102)
>> > org.apache.qpid.transport.network.Assembler.received(Assembler.java:44)
>> >
>> org.apache.qpid.transport.network.InputHandler.next(InputHandler.java:189)
>> >
>> >
>> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:105)
>> >
>> >
>> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:44)
>> > org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:152)
>> > java.lang.Thread.run(Thread.java:745)
>> >
>> > Session dispatcher thread
>> >
>> >
>> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1255)
>> >  ---------> waiting for connection's failoverMutex
>> >
>> >
>> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
>> > org.apache.qpid.client.AMQSession_0_10.sync(AMQSession_0_10.java:1034)
>> >
>> >
>> org.apache.qpid.client.AMQSession_0_10.sendSuspendChannel(AMQSession_0_10.java:812)
>> > org.apache.qpid.client.AMQSession.suspendChannel(AMQSession.java:3075)
>> > org.apache.qpid.client.AMQSession.rollback(AMQSession.java:1837)
>> > common.messaging.QpidSession.rollback(QpidSession.java:211)
>> >
>> >
>> common.messaging.QpidMessageHandler.rollbackSession(QpidMessageHandler.java:284)
>> >
>> common.messaging.QpidMessageHandler.onMessage(QpidMessageHandler.java:113)
>> >
>> >
>> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:748)
>> >
>> >
>> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:141)
>> >
>> >
>> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:722)
>> >
>> >
>> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:186)
>> >
>> >
>> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:54)
>> >
>> >
>> org.apache.qpid.client.AMQSession$Dispatcher.notifyConsumer(AMQSession.java:3454)
>> >
>> >
>> org.apache.qpid.client.AMQSession$Dispatcher.dispatchMessage(AMQSession.java:3393)
>> >   -----------> acquires session's messageDeliverylock
>> >
>> >
>> org.apache.qpid.client.AMQSession$Dispatcher.access$1000(AMQSession.java:3180)
>> > org.apache.qpid.client.AMQSession.dispatch(AMQSession.java:3173)
>> >
>> >
>> org.apache.qpid.client.message.UnprocessedMessage.dispatch(UnprocessedMessage.java:54)
>> > org.apache.qpid.client.AMQSession$Dispatcher.run(AMQSession.java:3316)
>> > java.lang.Thread.run(Thread.java:745)
>> >
>> > The problem is that the IOReceiver thread acquires failoverMutex before
>> > messageDeliveryLock (for each session), whereas the dispatcher thread
>> > acquires it in the other order. We also see potential problems where
>> other
>> > threads (instead of IOReceiver) can deadlock with the dispatcher
>> thread, as
>> > long as it acquires failoverMutex before messageDeliveryLock. Examples
>> we
>> > can think of:
>> >
>> > A) Another thread calling AMQSession.close()
>> > B) Another thread calling BasicMessageConsumer.close()
>> > C) Same connection, different session's dispatcher thread, calling
>> > rollback() or commit() -> sync() -> setCurrentException() ->
>> > AMQConnection.exceptionReceived() -> AMQConnection.closeAllSessions(),
>> > which can try to acquire the messageDeliveryLock of another session and
>> > deadlock with the other session's dispatcher thread
>> >
>> >
>> > Deadlock Variety 2:
>> > From code inspection, it also appears that AMQConnection.close() can
>> > deadlock with either AMQSession.close() or BasicMessageConsumer.close()
>> > (where the session / consumer is on the same connection). This is
>> because
>> > AMQConnection.close() first acquires the messageDeliveryLock of all its
>> > sessions in the recursive doClose(), before trying to acquire the
>> > connection's failoverMutex. But the Session / consumer's close()
>> acquires
>> > the failoverMutex before messageDeliveryLock. We haven't seen this
>> happen
>> > but would like to know if this is possible.
>> >
>> >
>> > We'd really appreciate your help on this. Assuming these can be fixed in
>> > 0.32, we are also wondering if clients are backward compatible -- i.e.,
>> can
>> > we upgrade only our client to 0.32 while continuing to use the 0.16
>> broker?
>> >
>> > Thanks,
>> > Helen
>> >
>>
>
>

Re: Qpid Java client deadlocks

Posted by Rajith Muditha Attapattu <ra...@gmail.com>.
The consensus is, that there is no easy fix, other than rewriting some key
pieces of the client.
Given that a new JMS client is being actively developed, we have decided
not to devote any significant time/resources on the older client.

The new JMS client is AMQP 1.0 and the source is here
https://git-wip-us.apache.org/repos/asf?p=qpid-jms.git
Robbie is the best person to give an accurate picture of the status for
this project.

Rajith

On Thu, Feb 5, 2015 at 2:42 AM, Rob Godfrey <ro...@gmail.com> wrote:

> To answer the easy question first, 0.32 clients should be totally
> compatible with an 0.16 broker.
>
> In terms of the deadlock issue, I know there are a number of open deadlock
> JIRAs that Robbie and Rajith have looked at in the past... Hopefully one of
> them will be able to chip in here and discuss the feasibility of fixing
> your particular issue for 0.32.
>
> -- Rob
>
> On 4 February 2015 at 23:36, Helen Kwong <he...@gmail.com> wrote:
>
> > Hi Qpid gurus,
> >
> > We are using 0.16 Java broker and client on 0-10, and we are running into
> > deadlock issues on the client that involve AMQSession's
> > _messageDeliveryLock and AMQConnection's _failoverMutex, where different
> > threads acquire them in different orders. This is leading to major
> > production headaches for us and has us very worried.
> >
> > (I've been looking at 0.16 code mostly but also skimmed the relevant
> parts
> > in 0.32, which seem largely the same in those places.)
> >
> > Deadlock Variety 1
> >
> > This is an example of a deadlock we see, where the IOReceiver thread
> > deadlocks with Session dispatcher thread (we have listeners that call
> > session rollback or commit in onMessage()):
> >
> > IOReceiver
> > org.apache.qpid.client.AMQSession.closed(AMQSession.java:818)
> ---------->
> > waiting for session's messageDeliveryLock
> >
> >
> org.apache.qpid.client.AMQConnection.closeAllSessions(AMQConnection.java:938)
> >
> >
> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1282)
> >  ----------> acquires connection's failoverMutex
> >
> >
> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
> >
> org.apache.qpid.client.AMQSession_0_10.exception(AMQSession_0_10.java:907)
> >
> >
> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:182)
> >
> >
> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:32)
> >
> >
> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
> >
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:55)
> >
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:50)
> >
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:32)
> > org.apache.qpid.transport.Method.delegate(Method.java:159)
> > org.apache.qpid.transport.Session.received(Session.java:585)
> > org.apache.qpid.transport.Connection.dispatch(Connection.java:412)
> >
> >
> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:64)
> >
> >
> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:40)
> >
> >
> org.apache.qpid.transport.MethodDelegate.executionException(MethodDelegate.java:110)
> >
> >
> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
> >
> >
> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:54)
> >
> >
> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:40)
> > org.apache.qpid.transport.Method.delegate(Method.java:159)
> > org.apache.qpid.transport.Connection.received(Connection.java:367)
> > org.apache.qpid.transport.Connection.received(Connection.java:65)
> > org.apache.qpid.transport.network.Assembler.emit(Assembler.java:97)
> > org.apache.qpid.transport.network.Assembler.assemble(Assembler.java:198)
> > org.apache.qpid.transport.network.Assembler.frame(Assembler.java:131)
> > org.apache.qpid.transport.network.Frame.delegate(Frame.java:128)
> > org.apache.qpid.transport.network.Assembler.received(Assembler.java:102)
> > org.apache.qpid.transport.network.Assembler.received(Assembler.java:44)
> >
> org.apache.qpid.transport.network.InputHandler.next(InputHandler.java:189)
> >
> >
> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:105)
> >
> >
> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:44)
> > org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:152)
> > java.lang.Thread.run(Thread.java:745)
> >
> > Session dispatcher thread
> >
> >
> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1255)
> >  ---------> waiting for connection's failoverMutex
> >
> >
> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
> > org.apache.qpid.client.AMQSession_0_10.sync(AMQSession_0_10.java:1034)
> >
> >
> org.apache.qpid.client.AMQSession_0_10.sendSuspendChannel(AMQSession_0_10.java:812)
> > org.apache.qpid.client.AMQSession.suspendChannel(AMQSession.java:3075)
> > org.apache.qpid.client.AMQSession.rollback(AMQSession.java:1837)
> > common.messaging.QpidSession.rollback(QpidSession.java:211)
> >
> >
> common.messaging.QpidMessageHandler.rollbackSession(QpidMessageHandler.java:284)
> >
> common.messaging.QpidMessageHandler.onMessage(QpidMessageHandler.java:113)
> >
> >
> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:748)
> >
> >
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:141)
> >
> >
> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:722)
> >
> >
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:186)
> >
> >
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:54)
> >
> >
> org.apache.qpid.client.AMQSession$Dispatcher.notifyConsumer(AMQSession.java:3454)
> >
> >
> org.apache.qpid.client.AMQSession$Dispatcher.dispatchMessage(AMQSession.java:3393)
> >   -----------> acquires session's messageDeliverylock
> >
> >
> org.apache.qpid.client.AMQSession$Dispatcher.access$1000(AMQSession.java:3180)
> > org.apache.qpid.client.AMQSession.dispatch(AMQSession.java:3173)
> >
> >
> org.apache.qpid.client.message.UnprocessedMessage.dispatch(UnprocessedMessage.java:54)
> > org.apache.qpid.client.AMQSession$Dispatcher.run(AMQSession.java:3316)
> > java.lang.Thread.run(Thread.java:745)
> >
> > The problem is that the IOReceiver thread acquires failoverMutex before
> > messageDeliveryLock (for each session), whereas the dispatcher thread
> > acquires it in the other order. We also see potential problems where
> other
> > threads (instead of IOReceiver) can deadlock with the dispatcher thread,
> as
> > long as it acquires failoverMutex before messageDeliveryLock. Examples we
> > can think of:
> >
> > A) Another thread calling AMQSession.close()
> > B) Another thread calling BasicMessageConsumer.close()
> > C) Same connection, different session's dispatcher thread, calling
> > rollback() or commit() -> sync() -> setCurrentException() ->
> > AMQConnection.exceptionReceived() -> AMQConnection.closeAllSessions(),
> > which can try to acquire the messageDeliveryLock of another session and
> > deadlock with the other session's dispatcher thread
> >
> >
> > Deadlock Variety 2:
> > From code inspection, it also appears that AMQConnection.close() can
> > deadlock with either AMQSession.close() or BasicMessageConsumer.close()
> > (where the session / consumer is on the same connection). This is because
> > AMQConnection.close() first acquires the messageDeliveryLock of all its
> > sessions in the recursive doClose(), before trying to acquire the
> > connection's failoverMutex. But the Session / consumer's close() acquires
> > the failoverMutex before messageDeliveryLock. We haven't seen this happen
> > but would like to know if this is possible.
> >
> >
> > We'd really appreciate your help on this. Assuming these can be fixed in
> > 0.32, we are also wondering if clients are backward compatible -- i.e.,
> can
> > we upgrade only our client to 0.32 while continuing to use the 0.16
> broker?
> >
> > Thanks,
> > Helen
> >
>

Re: Qpid Java client deadlocks

Posted by Rob Godfrey <ro...@gmail.com>.
To answer the easy question first, 0.32 clients should be totally
compatible with an 0.16 broker.

In terms of the deadlock issue, I know there are a number of open deadlock
JIRAs that Robbie and Rajith have looked at in the past... Hopefully one of
them will be able to chip in here and discuss the feasibility of fixing
your particular issue for 0.32.

-- Rob

On 4 February 2015 at 23:36, Helen Kwong <he...@gmail.com> wrote:

> Hi Qpid gurus,
>
> We are using 0.16 Java broker and client on 0-10, and we are running into
> deadlock issues on the client that involve AMQSession's
> _messageDeliveryLock and AMQConnection's _failoverMutex, where different
> threads acquire them in different orders. This is leading to major
> production headaches for us and has us very worried.
>
> (I've been looking at 0.16 code mostly but also skimmed the relevant parts
> in 0.32, which seem largely the same in those places.)
>
> Deadlock Variety 1
>
> This is an example of a deadlock we see, where the IOReceiver thread
> deadlocks with Session dispatcher thread (we have listeners that call
> session rollback or commit in onMessage()):
>
> IOReceiver
> org.apache.qpid.client.AMQSession.closed(AMQSession.java:818)  ---------->
> waiting for session's messageDeliveryLock
>
> org.apache.qpid.client.AMQConnection.closeAllSessions(AMQConnection.java:938)
>
> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1282)
>  ----------> acquires connection's failoverMutex
>
> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
> org.apache.qpid.client.AMQSession_0_10.exception(AMQSession_0_10.java:907)
>
> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:182)
>
> org.apache.qpid.transport.SessionDelegate.executionException(SessionDelegate.java:32)
>
> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:55)
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:50)
> org.apache.qpid.transport.SessionDelegate.command(SessionDelegate.java:32)
> org.apache.qpid.transport.Method.delegate(Method.java:159)
> org.apache.qpid.transport.Session.received(Session.java:585)
> org.apache.qpid.transport.Connection.dispatch(Connection.java:412)
>
> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:64)
>
> org.apache.qpid.transport.ConnectionDelegate.handle(ConnectionDelegate.java:40)
>
> org.apache.qpid.transport.MethodDelegate.executionException(MethodDelegate.java:110)
>
> org.apache.qpid.transport.ExecutionException.dispatch(ExecutionException.java:103)
>
> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:54)
>
> org.apache.qpid.transport.ConnectionDelegate.command(ConnectionDelegate.java:40)
> org.apache.qpid.transport.Method.delegate(Method.java:159)
> org.apache.qpid.transport.Connection.received(Connection.java:367)
> org.apache.qpid.transport.Connection.received(Connection.java:65)
> org.apache.qpid.transport.network.Assembler.emit(Assembler.java:97)
> org.apache.qpid.transport.network.Assembler.assemble(Assembler.java:198)
> org.apache.qpid.transport.network.Assembler.frame(Assembler.java:131)
> org.apache.qpid.transport.network.Frame.delegate(Frame.java:128)
> org.apache.qpid.transport.network.Assembler.received(Assembler.java:102)
> org.apache.qpid.transport.network.Assembler.received(Assembler.java:44)
> org.apache.qpid.transport.network.InputHandler.next(InputHandler.java:189)
>
> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:105)
>
> org.apache.qpid.transport.network.InputHandler.received(InputHandler.java:44)
> org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:152)
> java.lang.Thread.run(Thread.java:745)
>
> Session dispatcher thread
>
> org.apache.qpid.client.AMQConnection.exceptionReceived(AMQConnection.java:1255)
>  ---------> waiting for connection's failoverMutex
>
> org.apache.qpid.client.AMQSession_0_10.setCurrentException(AMQSession_0_10.java:1057)
> org.apache.qpid.client.AMQSession_0_10.sync(AMQSession_0_10.java:1034)
>
> org.apache.qpid.client.AMQSession_0_10.sendSuspendChannel(AMQSession_0_10.java:812)
> org.apache.qpid.client.AMQSession.suspendChannel(AMQSession.java:3075)
> org.apache.qpid.client.AMQSession.rollback(AMQSession.java:1837)
> common.messaging.QpidSession.rollback(QpidSession.java:211)
>
> common.messaging.QpidMessageHandler.rollbackSession(QpidMessageHandler.java:284)
> common.messaging.QpidMessageHandler.onMessage(QpidMessageHandler.java:113)
>
> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:748)
>
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:141)
>
> org.apache.qpid.client.BasicMessageConsumer.notifyMessage(BasicMessageConsumer.java:722)
>
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:186)
>
> org.apache.qpid.client.BasicMessageConsumer_0_10.notifyMessage(BasicMessageConsumer_0_10.java:54)
>
> org.apache.qpid.client.AMQSession$Dispatcher.notifyConsumer(AMQSession.java:3454)
>
> org.apache.qpid.client.AMQSession$Dispatcher.dispatchMessage(AMQSession.java:3393)
>   -----------> acquires session's messageDeliverylock
>
> org.apache.qpid.client.AMQSession$Dispatcher.access$1000(AMQSession.java:3180)
> org.apache.qpid.client.AMQSession.dispatch(AMQSession.java:3173)
>
> org.apache.qpid.client.message.UnprocessedMessage.dispatch(UnprocessedMessage.java:54)
> org.apache.qpid.client.AMQSession$Dispatcher.run(AMQSession.java:3316)
> java.lang.Thread.run(Thread.java:745)
>
> The problem is that the IOReceiver thread acquires failoverMutex before
> messageDeliveryLock (for each session), whereas the dispatcher thread
> acquires it in the other order. We also see potential problems where other
> threads (instead of IOReceiver) can deadlock with the dispatcher thread, as
> long as it acquires failoverMutex before messageDeliveryLock. Examples we
> can think of:
>
> A) Another thread calling AMQSession.close()
> B) Another thread calling BasicMessageConsumer.close()
> C) Same connection, different session's dispatcher thread, calling
> rollback() or commit() -> sync() -> setCurrentException() ->
> AMQConnection.exceptionReceived() -> AMQConnection.closeAllSessions(),
> which can try to acquire the messageDeliveryLock of another session and
> deadlock with the other session's dispatcher thread
>
>
> Deadlock Variety 2:
> From code inspection, it also appears that AMQConnection.close() can
> deadlock with either AMQSession.close() or BasicMessageConsumer.close()
> (where the session / consumer is on the same connection). This is because
> AMQConnection.close() first acquires the messageDeliveryLock of all its
> sessions in the recursive doClose(), before trying to acquire the
> connection's failoverMutex. But the Session / consumer's close() acquires
> the failoverMutex before messageDeliveryLock. We haven't seen this happen
> but would like to know if this is possible.
>
>
> We'd really appreciate your help on this. Assuming these can be fixed in
> 0.32, we are also wondering if clients are backward compatible -- i.e., can
> we upgrade only our client to 0.32 while continuing to use the 0.16 broker?
>
> Thanks,
> Helen
>