-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5794/
-----------------------------------------------------------

Review request for qpid, Robbie Gemmell, Keith Wall, and Rob Godfrey.


Description
-------

When a session exception is received, we notify the connection via the 
exceptionReceived method. The connection will close all sessions for this 
connection, but does not close the underlying Protocol connection.
Further calls (from application code) to close the connection, does not close 
the underlying protocol-connection as it skips this part, due to the connection 
already being marked as closed.
This results in leaked connections.

The proposed patch does the following.
1. When a session exception is received, it closes the Session (and the 
consumers/producers associated with it) to prevent further use of this session.
2. It also notifies the connection, however the exception being passed, is 
marked as a soft error, thereby preventing the connection from being marked as 
closed.
   It also prevents the rest of the sessions being closed.
3. The exception will however be notified via the exception listener. This is 
important in certain cases, as it might be the only case where an exception can 
be notified.

What is the impact to applications.
------------------------------------
1. If an application closes the connection when it receives an exception, and 
then recreates the connection and it's associated objects 
(sessions/consumers/producers) then this change will have no impact.

2. If an application just recreates everything from scratch, without resorting 
to closing the connection (assuming that all exceptions received via the 
listener will automatically close the connection), there will be tcp connection 
leaks. 
However the objects associated with the connection will eventualy be garbage 
collected. We should probably close the underlying TCP connection in the 
finalizer for the Connection object as a safety measure.

With this change, the recommended approach would be, for application to check 
the connection status in deciding what it should do.
For example, 

if (conn.isClosed())
{
   // re-create connection.
}
else
{
  // session exception
  // this means the rest of the sessions and the connection is still open.
}


This addresses bug QPID-3575.
    https://issues.apache.org/jira/browse/QPID-3575


Diffs
-----

  
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java
 1357981 
  
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java
 1357981 
  
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/AMQException.java
 1357981 

Diff: https://reviews.apache.org/r/5794/diff/


Testing
-------

Tested with the default java profile and the cpp test profile.

SelectorTest#testRuntimeSelectorError is failing as it expects the connection 
to be closed.
I need to investigate further to determine if the test needs to be changed or 
not.


Thanks,

rajith attapattu

Reply via email to