> On 2011-11-15 16:41:28, Gordon Sim wrote: > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java, > > line 787 > > <https://reviews.apache.org/r/2832/diff/1/?file=58392#file58392line787> > > > > Are there any other locks acquired as part of the block here? If so are > > there any lock ordering issues where you could be introducing a deadlock? > > rajith attapattu wrote: > Not that I could think of. The message-delivery-lock is taken to ensure > that no messages are being served while we start pulling them out of the > queue. > In my tests so far, I haven't encountered any issues. However I plan to > have more manual tests - ex. Trying to stop the connection while the message > consumers are in full flight. > > Gordon Sim wrote: > What about the failover mutex? Could the release trigger a codepath that > attempts to acquire that? What about an asynchronous exception occurring > concurrently; would that ever need to acquire the message-delivery-lock? > > rajith attapattu wrote: > Certainly possible as mentioned in the comment below. The failover and > the synchronous exceptions are things that could trigger a deadlock. > Testing is the best way to eliminate these possibilities. However IMO > acquiring the message-delivery-lock is a must to ensure unwanted interaction > between messages delivery & releasing.
Testing is certainly vital, however with threading issues a thorough analysis is as well given the non-deterministic nature of potential problems. - Gordon ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2832/#review3264 ----------------------------------------------------------- On 2011-11-15 15:36:36, rajith attapattu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2832/ > ----------------------------------------------------------- > > (Updated 2011-11-15 15:36:36) > > > Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and > Oleksandr Rudyy. > > > Summary > ------- > > This attempts to fix one of the issues related to the handling of Message > credits. See QPID-3602 for an overall picture of the various issues. > > This particular patch does the following. > 1. When the connection is stopped, it sends message.stop() & releases all > messages in the prefetch buffer. > 2. It will also release any messages (that were in flight) that comes after > the connection is stopped. (*) > > (*) This interferes with the immediate_prefetch feature. However I don't know > if immediate prefetch is really required in the 0-10 path. > > As always comments, suggestions & criticisms are equally welcomed. > > > This addresses bug QPID-3604. > https://issues.apache.org/jira/browse/QPID-3604 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java > 1202228 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java > 1202228 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/client/prefetch/PrefetchBehaviourTest.java > 1202228 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java > 1202228 > > Diff: https://reviews.apache.org/r/2832/diff > > > Testing > ------- > > See PrefetchBehaviourTest#testConnectionStop for more details. > > > Thanks, > > rajith > >
