From 88ca352ffee7df7e037bed1e73936e9b5d9b9ae2 Mon Sep 17 00:00:00 2001
From: Keith Wall <kwall@apache.org>
Date: Fri, 16 Aug 2013 11:54:02 +0000
Subject: 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
---
 .../java/org/apache/qpid/client/AMQConnection.java | 76 +++++++++++++---------
 1 file changed, 44 insertions(+), 32 deletions(-)

(limited to 'qpid/java/client/src/main')

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)
-- 
cgit v1.2.1