QPID-5050: Move invocation of ExceptionListener to after the failoverMutex is released avoiding deadlock possibility

Previously, the ExceptionListener was invoked whilst the failoverMutex was held, between the
two potential state changes (connection state change and session state change).

This commit reorders the statements so that the ExceptionListner is fired after the failoverMutex
is released. It also means that the ExceptionListener is fired *after* both connection/session
have undergone any state changes. The exceptionListener member is also made thread safe.

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1514664 13f79535-47bb-0310-9956-ffa450edef68

1 files changed, 44 insertions, 32 deletions

diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
index 74c9878a8e..aadfb44ca0 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
@@ -126,7 +126,8 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
     /** The virtual path to connect to on the AMQ server */
     private String _virtualHost;
 
-    private ExceptionListener _exceptionListener;
+    /** The exception listener for this connection object. */
+    private volatile ExceptionListener _exceptionListener;
 
     private ConnectionListener _connectionListener;
 
@@ -784,13 +785,13 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
     public ExceptionListener getExceptionListener() throws JMSException
     {
         checkNotClosed();
-
-        return _exceptionListener;
+        return getExceptionListenerNoCheck();
     }
 
     public void setExceptionListener(ExceptionListener listener) throws JMSException
     {
         checkNotClosed();
+
         _exceptionListener = listener;
     }
 
@@ -1307,45 +1308,56 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
             _protocolHandler.getProtocolSession().notifyError(je);
         }
 
-        // get the failover mutex before trying to close
-        synchronized (getFailoverMutex())
+        try
         {
-            // decide if we are going to close the session
-            if (hardError(cause))
+            // get the failover mutex before trying to close
+            synchronized (getFailoverMutex())
             {
-                closer = (!setClosed()) || closer;
+                // decide if we are going to close the session
+                if (hardError(cause))
                 {
-                    _logger.info("Closing AMQConnection due to :" + cause);
+                    closer = (!setClosed()) || closer;
+                    {
+                        _logger.info("Closing AMQConnection due to :" + cause);
+                    }
                 }
-            }
-            else
-            {
-                _logger.info("Not a hard-error connection not closing: " + cause);
-            }
-
-            // deliver the exception if there is a listener
-            if (_exceptionListener != null)
-            {
-                _exceptionListener.onException(je);
-            }
-            else
-            {
-                _logger.error("Throwable Received but no listener set: " + cause);
-            }
-
-            // if we are closing the connection, close sessions first
-            if (closer)
-            {
-                try
+                else
                 {
-                    closeAllSessions(cause, -1, -1); // FIXME: when doing this end up with RejectedExecutionException from executor.
+                    _logger.info("Not a hard-error connection not closing: " + cause);
                 }
-                catch (JMSException e)
+
+                // if we are closing the connection, close sessions first
+                if (closer)
                 {
-                    _logger.error("Error closing all sessions: " + e, e);
+                    try
+                    {
+                        closeAllSessions(cause, -1, -1); // FIXME: when doing this end up with RejectedExecutionException from executor.
+                    }
+                    catch (JMSException e)
+                    {
+                        _logger.error("Error closing all sessions: " + e, e);
+                    }
                 }
             }
         }
+        finally
+        {
+            deliverJMSExceptionToExceptionListenerOrLog(je, cause);
+        }
+    }
+
+    private void deliverJMSExceptionToExceptionListenerOrLog(final JMSException je, final Throwable cause)
+    {
+        // deliver the exception if there is a listener
+        ExceptionListener exceptionListener = getExceptionListenerNoCheck();
+        if (exceptionListener != null)
+        {
+            exceptionListener.onException(je);
+        }
+        else
+        {
+            _logger.error("Throwable Received but no listener set: " + cause);
+        }
     }
 
     private boolean hardError(Throwable cause)