ARTEMIS-1305 Server Frozen during shutdown if RA is retrying

artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java

@@ -1362,7 +1362,12 @@ public final class ServerLocatorImpl implements ServerLocatorInternal, Discovery
       }
       for (ClientSessionFactory factory : clonedFactory) {
          if (sendClose) {
-            factory.close();
+            try {
+               factory.close();
+            } catch (Throwable e) {
+               logger.debug(e.getMessage(), e);
+               factory.cleanup();
+            }
          } else {
             factory.cleanup();
          }

artemis-ra/src/main/java/org/apache/activemq/artemis/ra/ActiveMQRAManagedConnection.java

@@ -55,16 +55,14 @@ import org.apache.activemq.artemis.jms.client.ActiveMQXAConnection;
 import org.apache.activemq.artemis.service.extensions.ServiceUtils;
 import org.apache.activemq.artemis.service.extensions.xa.ActiveMQXAResourceWrapper;
 import org.apache.activemq.artemis.utils.VersionLoader;
+import org.jboss.logging.Logger;
 
 /**
  * The managed connection
  */
 public final class ActiveMQRAManagedConnection implements ManagedConnection, ExceptionListener {
 
-   /**
+   private static final Logger logger = Logger.getLogger(ActiveMQRAManagedConnection.class);
-    * Trace enabled
-    */
-   private static boolean trace = ActiveMQRALogger.LOGGER.isTraceEnabled();
 
    /**
     * The managed connection factory
@@ -141,7 +139,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
                                       final ActiveMQResourceAdapter ra,
                                       final String userName,
                                       final String password) throws ResourceException {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("constructor(" + mcf + ", " + cri + ", " + userName + ", ****)");
       }
 
@@ -188,7 +186,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
    @Override
    public synchronized Object getConnection(final Subject subject,
                                             final ConnectionRequestInfo cxRequestInfo) throws ResourceException {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("getConnection(" + subject + ", " + cxRequestInfo + ")");
       }
 
@@ -219,19 +217,10 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     * @throws ResourceException Failed to close one or more handles.
     */
    private void destroyHandles() throws ResourceException {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("destroyHandles()");
       }
 
-      try {
-
-         if (connection != null) {
-            connection.stop();
-         }
-      } catch (Throwable t) {
-         ActiveMQRALogger.LOGGER.trace("Ignored error stopping connection", t);
-      }
-
       for (ActiveMQRASession session : handles) {
          session.destroy();
       }
@@ -246,7 +235,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     */
    @Override
    public void destroy() throws ResourceException {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("destroy()");
       }
 
@@ -259,41 +248,21 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
       try {
          connection.setExceptionListener(null);
       } catch (JMSException e) {
-         ActiveMQRALogger.LOGGER.debug("Error unsetting the exception listener " + this, e);
+         logger.debug("Error unsetting the exception listener " + this, e);
       }
       if (connection != null) {
          connection.signalStopToAllSessions();
       }
 
+      try {
+         connectionFactory.close();
+      } catch (Exception e) {
+         logger.debug(e.getMessage(), e);
+      }
+
       destroyHandles();
 
       try {
-         /**
-          * (xa|nonXA)Session.close() may NOT be called BEFORE connection.close()
-          * <p>
-          * If the ClientSessionFactory is trying to fail-over or reconnect with -1 attempts, and
-          * one calls session.close() it may effectively dead-lock.
-          * <p>
-          * connection close will close the ClientSessionFactory which will close all sessions.
-          */
-         if (connection != null) {
-            connection.close();
-         }
-
-         // The following calls should not be necessary, as the connection should close the
-         // ClientSessionFactory, which will close the sessions.
-         try {
-            if (nonXAsession != null) {
-               nonXAsession.close();
-            }
-
-            if (xaSession != null) {
-               xaSession.close();
-            }
-         } catch (JMSException e) {
-            ActiveMQRALogger.LOGGER.debug("Error closing session " + this, e);
-         }
-
          // we must close the ActiveMQConnectionFactory because it contains a ServerLocator
          if (connectionFactory != null) {
             ra.closeConnectionFactory(mcf.getProperties());
@@ -310,7 +279,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     */
    @Override
    public void cleanup() throws ResourceException {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("cleanup()");
       }
 
@@ -340,7 +309,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     */
    @Override
    public void associateConnection(final Object obj) throws ResourceException {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("associateConnection(" + obj + ")");
       }
 
@@ -361,9 +330,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
             if (tx != null) {
                int status = tx.getStatus();
                // Only allow states that will actually succeed
-               if (status != Status.STATUS_ACTIVE && status != Status.STATUS_PREPARING &&
+               if (status != Status.STATUS_ACTIVE && status != Status.STATUS_PREPARING && status != Status.STATUS_PREPARED && status != Status.STATUS_COMMITTING) {
-                  status != Status.STATUS_PREPARED &&
-                  status != Status.STATUS_COMMITTING) {
                   throw new javax.jms.IllegalStateException("Transaction " + tx + " not active");
                }
             }
@@ -379,7 +346,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     * Aqquire a lock on the managed connection
     */
    protected void lock() {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("lock()");
       }
 
@@ -392,7 +359,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     * @throws JMSException Thrown if an error occurs
     */
    protected void tryLock() throws JMSException {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("tryLock()");
       }
 
@@ -414,7 +381,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     * Unlock the managed connection
     */
    protected void unlock() {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("unlock()");
       }
 
@@ -428,7 +395,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     */
    @Override
    public void addConnectionEventListener(final ConnectionEventListener l) {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("addConnectionEventListener(" + l + ")");
       }
 
@@ -442,7 +409,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     */
    @Override
    public void removeConnectionEventListener(final ConnectionEventListener l) {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("removeConnectionEventListener(" + l + ")");
       }
 
@@ -457,7 +424,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     */
    @Override
    public XAResource getXAResource() throws ResourceException {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("getXAResource()");
       }
 
@@ -476,7 +443,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
          xaResource = ServiceUtils.wrapXAResource(activeMQRAXAResource, xaResourceProperties);
       }
 
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("XAResource=" + xaResource);
       }
 
@@ -491,13 +458,13 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     */
    @Override
    public LocalTransaction getLocalTransaction() throws ResourceException {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("getLocalTransaction()");
       }
 
       LocalTransaction tx = new ActiveMQRALocalTransaction(this);
 
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("LocalTransaction=" + tx);
       }
 
@@ -513,7 +480,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     */
    @Override
    public ManagedConnectionMetaData getMetaData() throws ResourceException {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("getMetaData()");
       }
 
@@ -532,7 +499,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     */
    @Override
    public void setLogWriter(final PrintWriter out) throws ResourceException {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("setLogWriter(" + out + ")");
       }
    }
@@ -545,7 +512,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     */
    @Override
    public PrintWriter getLogWriter() throws ResourceException {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("getLogWriter()");
       }
 
@@ -562,12 +529,12 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
       if (ActiveMQConnection.EXCEPTION_FAILOVER.equals(exception.getErrorCode())) {
          return;
       }
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("onException(" + exception + ")");
       }
 
       if (isDestroyed.get()) {
-         if (ActiveMQRAManagedConnection.trace) {
+         if (logger.isTraceEnabled()) {
             ActiveMQRALogger.LOGGER.trace("Ignoring error on already destroyed connection " + this, exception);
          }
          return;
@@ -578,7 +545,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
       try {
          connection.setExceptionListener(null);
       } catch (JMSException e) {
-         ActiveMQRALogger.LOGGER.debug("Unable to unset exception listener", e);
+         logger.debug("Unable to unset exception listener", e);
       }
 
       ConnectionEvent event = new ConnectionEvent(this, ConnectionEvent.CONNECTION_ERROR_OCCURRED, exception);
@@ -593,13 +560,13 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     */
    protected Session getSession() throws JMSException {
       if (xaResource != null && inManagedTx) {
-         if (ActiveMQRAManagedConnection.trace) {
+         if (logger.isTraceEnabled()) {
             ActiveMQRALogger.LOGGER.trace("getSession() -> XA session " + xaSession.getSession());
          }
 
          return xaSession.getSession();
       } else {
-         if (ActiveMQRAManagedConnection.trace) {
+         if (logger.isTraceEnabled()) {
             ActiveMQRALogger.LOGGER.trace("getSession() -> non XA session " + nonXAsession);
          }
 
@@ -613,7 +580,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     * @param event The event to send.
     */
    protected void sendEvent(final ConnectionEvent event) {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("sendEvent(" + event + ")");
       }
 
@@ -656,7 +623,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     * @param handle The handle to remove.
     */
    protected void removeHandle(final ActiveMQRASession handle) {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("removeHandle(" + handle + ")");
       }
 
@@ -669,7 +636,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     * @return The connection request info for this connection.
     */
    protected ActiveMQRAConnectionRequestInfo getCRI() {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("getCRI()");
       }
 
@@ -682,7 +649,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     * @return The connection factory for this connection.
     */
    protected ActiveMQRAManagedConnectionFactory getManagedConnectionFactory() {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("getManagedConnectionFactory()");
       }
 
@@ -695,7 +662,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     * @throws JMSException Thrown if the connection can't be started
     */
    void start() throws JMSException {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("start()");
       }
 
@@ -710,7 +677,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     * @throws JMSException Thrown if the connection can't be stopped
     */
    void stop() throws JMSException {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("stop()");
       }
 
@@ -725,7 +692,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     * @return The user name
     */
    protected String getUserName() {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("getUserName()");
       }
 
@@ -738,7 +705,7 @@ public final class ActiveMQRAManagedConnection implements ManagedConnection, Exc
     * @throws ResourceException Thrown if a connection couldn't be created
     */
    private void setup() throws ResourceException {
-      if (ActiveMQRAManagedConnection.trace) {
+      if (logger.isTraceEnabled()) {
          ActiveMQRALogger.LOGGER.trace("setup()");
       }
 

artemis-ra/src/main/java/org/apache/activemq/artemis/ra/inflow/ActiveMQActivation.java

@@ -295,7 +295,7 @@ public class ActiveMQActivation {
     * @throws Exception Thrown if an error occurs
     */
    protected synchronized void setup() throws Exception {
-      ActiveMQRALogger.LOGGER.debug("Setting up " + spec);
+      logger.debug("Setting up " + spec);
 
       setupCF();
 
@@ -346,14 +346,14 @@ public class ActiveMQActivation {
          factory.getServerLocator().addClusterTopologyListener(new RebalancingListener());
       }
 
-      ActiveMQRALogger.LOGGER.debug("Setup complete " + this);
+      logger.debug("Setup complete " + this);
    }
 
    /**
     * Teardown the activation
     */
    protected synchronized void teardown() {
-      ActiveMQRALogger.LOGGER.debug("Tearing down " + spec);
+      logger.debug("Tearing down " + spec);
 
       long timeout = factory == null ? ActiveMQClient.DEFAULT_CALL_TIMEOUT : factory.getCallTimeout();
 
@@ -442,7 +442,7 @@ public class ActiveMQActivation {
       nodes.clear();
       lastReceived = false;
 
-      ActiveMQRALogger.LOGGER.debug("Tearing down complete " + this);
+      logger.debug("Tearing down complete " + this);
    }
 
    protected void setupCF() throws Exception {
@@ -486,7 +486,7 @@ public class ActiveMQActivation {
             result.addMetaData(ClientSession.JMS_SESSION_CLIENT_ID_PROPERTY, clientID);
          }
 
-         ActiveMQRALogger.LOGGER.debug("Using queue connection " + result);
+         logger.debug("Using queue connection " + result);
 
          return result;
       } catch (Throwable t) {
@@ -519,14 +519,14 @@ public class ActiveMQActivation {
          } else {
             ctx = new InitialContext(spec.getParsedJndiParams());
          }
-         ActiveMQRALogger.LOGGER.debug("Using context " + ctx.getEnvironment() + " for " + spec);
+         logger.debug("Using context " + ctx.getEnvironment() + " for " + spec);
          if (logger.isTraceEnabled()) {
             logger.trace("setupDestination(" + ctx + ")");
          }
 
          String destinationTypeString = spec.getDestinationType();
          if (destinationTypeString != null && !destinationTypeString.trim().equals("")) {
-            ActiveMQRALogger.LOGGER.debug("Destination type defined as " + destinationTypeString);
+            logger.debug("Destination type defined as " + destinationTypeString);
 
             Class<?> destinationType;
             if (Topic.class.getName().equals(destinationTypeString)) {
@@ -536,7 +536,7 @@ public class ActiveMQActivation {
                destinationType = Queue.class;
             }
 
-            ActiveMQRALogger.LOGGER.debug("Retrieving " + destinationType.getName() + " \"" + destinationName + "\" from JNDI");
+            logger.debug("Retrieving " + destinationType.getName() + " \"" + destinationName + "\" from JNDI");
 
             try {
                destination = (ActiveMQDestination) ActiveMQRaUtils.lookup(ctx, destinationName, destinationType);
@@ -547,7 +547,7 @@ public class ActiveMQActivation {
 
                String calculatedDestinationName = destinationName.substring(destinationName.lastIndexOf('/') + 1);
 
-               ActiveMQRALogger.LOGGER.debug("Unable to retrieve " + destinationName +
+               logger.debug("Unable to retrieve " + destinationName +
                                                 " from JNDI. Creating a new " + destinationType.getName() +
                                                 " named " + calculatedDestinationName + " to be used by the MDB.");
 
@@ -559,8 +559,8 @@ public class ActiveMQActivation {
                }
             }
          } else {
-            ActiveMQRALogger.LOGGER.debug("Destination type not defined in MDB activation configuration.");
+            logger.debug("Destination type not defined in MDB activation configuration.");
-            ActiveMQRALogger.LOGGER.debug("Retrieving " + Destination.class.getName() + " \"" + destinationName + "\" from JNDI");
+            logger.debug("Retrieving " + Destination.class.getName() + " \"" + destinationName + "\" from JNDI");
 
             destination = (ActiveMQDestination) ActiveMQRaUtils.lookup(ctx, destinationName, Destination.class);
             if (destination instanceof Topic) {
@@ -646,7 +646,7 @@ public class ActiveMQActivation {
             try {
                Thread.sleep(setupInterval);
             } catch (InterruptedException e) {
-               ActiveMQRALogger.LOGGER.debug("Interrupted trying to reconnect " + spec, e);
+               logger.debug("Interrupted trying to reconnect " + spec, e);
                break;
             }