> On 2011-04-21 08:39:41, Gordon Sim wrote: > > That's a suggested fix for QPID-3214 right (not QPID-3216)? > > > > I'm not that familiar with the code, but based on point 1 in your comment > > on QPID-3214 I have a few questions: > > > > Not signalling session exceptions to the listener would be a semantic > > change, right? Might some applications be relying on that at present? Might > > there be cases where that was actually the only way to get informed of a > > session issue? E.g. if a session just has a message listener set and if > > the/a queue subscribed to is then deleted, a not-found exception will be > > sent by the broker. Will the application detect that?
Sorry it was a typo - yes it should be QPID-3214. I have modified the title and the description. As mentioned in the JIRA, the exception handling is wrong! It clearly violates both the JMS spec as well as the AMQP spec. 1. The JMS API clearly states that only "serious problem with a Connection object" should be notified via the exception listener. A session closed (for ex. due to resource exhaustion or ACL violation) should not close the rest of the sessions along with the connection. 2. The AMQP spec states that execution exceptions are soft errors and should not result in the connection (and the rest of the sessions) being closed. While I agree that my suggested patch would be a semantic change, it is indeed the correct behaviour. I did some testing to see if we report session exceptions properly (in the 0-10 code path and this patch only affects 0-10). It seems we do throw a JMS exception on a create producer or consumer if there is an ACL violation. All actions in JMS (exception for message producing unless explicitly marked synchronous) is synchronous and we do throw a JMS exception if something goes wrong. We do throw a JMS exception on send method if the queue runs out of space, however due to the async nature it will be reported on a subsequent send. However if we send just one more message over the limit the session does get closed, but an exception is not thrown for some subsequent operations on the closed session. Ex creating a producer. This is a bug as there is no check to see if the session is alive when creating a producer. If we can fix those minor issues, then I strongly suggest that my patch is the way to go as it restore the correct behaviour. - rajith ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/642/#review519 ----------------------------------------------------------- On 2011-04-20 22:14:56, rajith attapattu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/642/ > ----------------------------------------------------------- > > (Updated 2011-04-20 22:14:56) > > > Review request for qpid. > > > Summary > ------- > > Suggested fix for https://issues.apache.org/jira/browse/QPID-3216 > Please note we may also need to fix the exceptionReceived method in > AMQConnection.java for reasons outlined in the above JIRA. > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java > 1095508 > > Diff: https://reviews.apache.org/r/642/diff > > > Testing > ------- > > > Thanks, > > rajith > >
