From 046c3b19422c821136c36bf65f65e674e071a44f Mon Sep 17 00:00:00 2001 From: Martyn Taylor Date: Wed, 4 Nov 2015 12:53:39 +0000 Subject: [PATCH] ARTEMIS-297 Handle Exceptions during server stop Stopping the server should be able to handle exceptions thrown by individual components. Before this patch the stop method would throw any raised exception and exit. This can lead to partial shutdowns of the server resulting in leaking threads. This patch catches any component exceptions and logs an appropriate error message, allowing the server to properly shutdown. --- .../server/impl/RemotingServiceImpl.java | 17 +++- .../core/server/ActiveMQServerLogger.java | 8 ++ .../core/server/impl/ActiveMQServerImpl.java | 92 ++++++++++++++----- 3 files changed, 93 insertions(+), 24 deletions(-) diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java index 42752bc0d7..d3bfdb1ad1 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java @@ -346,7 +346,14 @@ public class RemotingServiceImpl implements RemotingService, ConnectionLifeCycle if (ActiveMQServerLogger.LOGGER.isDebugEnabled()) { ActiveMQServerLogger.LOGGER.debug("Pausing acceptor " + acceptor); } - acceptor.pause(); + + try { + acceptor.pause(); + } + catch (Throwable t) { + ActiveMQServerLogger.LOGGER.errorStoppingAcceptor(); + } + } if (ActiveMQServerLogger.LOGGER.isDebugEnabled()) { @@ -368,7 +375,12 @@ public class RemotingServiceImpl implements RemotingService, ConnectionLifeCycle } for (Acceptor acceptor : acceptors.values()) { - acceptor.stop(); + try { + acceptor.stop(); + } + catch (Throwable t) { + ActiveMQServerLogger.LOGGER.errorStoppingAcceptor(); + } } acceptors.clear(); @@ -391,7 +403,6 @@ public class RemotingServiceImpl implements RemotingService, ConnectionLifeCycle } started = false; - } @Override diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java index 250eb2d95d..be167ecdd7 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java @@ -1457,4 +1457,12 @@ public interface ActiveMQServerLogger extends BasicLogger { @LogMessage(level = Logger.Level.ERROR) @Message(id = 224067, value = "Error populating security roles from LDAP", format = Message.Format.MESSAGE_FORMAT) void errorPopulatingSecurityRolesFromLDAP(@Cause Exception e); + + @LogMessage(level = Logger.Level.ERROR) + @Message(id = 224068, value = "Unable to stop component: {0}", format = Message.Format.MESSAGE_FORMAT) + void errorStoppingComponent(@Cause Throwable t, String componentClassName); + + @LogMessage(level = Logger.Level.DEBUG) + @Message(id = 224070, value = "Received Interrupt Exception whilst waiting for component to shutdown: {0}", format = Message.Format.MESSAGE_FORMAT) + void interruptWhilstStoppingComponent(String componentClassName); } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java index e7d4c5f0eb..8b5a9bf87f 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java @@ -572,7 +572,7 @@ public class ActiveMQServerImpl implements ActiveMQServer { * * @param criticalIOError whether we have encountered an IO error with the journal etc */ - void stop(boolean failoverOnServerShutdown, final boolean criticalIOError, boolean restarting) throws Exception { + void stop(boolean failoverOnServerShutdown, final boolean criticalIOError, boolean restarting) { synchronized (this) { if (state == SERVER_STATE.STOPPED || state == SERVER_STATE.STOPPING) { return; @@ -587,7 +587,7 @@ public class ActiveMQServerImpl implements ActiveMQServer { // aren't removed in case of failover if (groupingHandler != null) { managementService.removeNotificationListener(groupingHandler); - groupingHandler.stop(); + stopComponent(groupingHandler); } stopComponent(clusterManager); @@ -598,11 +598,16 @@ public class ActiveMQServerImpl implements ActiveMQServer { // allows for graceful shutdown if (remotingService != null && configuration.isGracefulShutdownEnabled()) { long timeout = configuration.getGracefulShutdownTimeout(); - if (timeout == -1) { - remotingService.getConnectionCountLatch().await(); + try { + if (timeout == -1) { + remotingService.getConnectionCountLatch().await(); + } + else { + remotingService.getConnectionCountLatch().await(timeout); + } } - else { - remotingService.getConnectionCountLatch().await(timeout); + catch (InterruptedException e) { + ActiveMQServerLogger.LOGGER.interruptWhilstStoppingComponent(remotingService.getClass().getName()); } } @@ -629,24 +634,45 @@ public class ActiveMQServerImpl implements ActiveMQServer { callDeActiveCallbacks(); stopComponent(backupManager); - activation.preStorageClose(); + + try { + activation.preStorageClose(); + } + catch (Throwable t) { + ActiveMQServerLogger.LOGGER.errorStoppingComponent(t, activation.getClass().getName()); + } + stopComponent(pagingManager); if (storageManager != null) - storageManager.stop(criticalIOError); + try { + storageManager.stop(criticalIOError); + } + catch (Throwable t) { + ActiveMQServerLogger.LOGGER.errorStoppingComponent(t, storageManager.getClass().getName()); + } // We stop remotingService before otherwise we may lock the system in case of a critical IO // error shutdown if (remotingService != null) - remotingService.stop(criticalIOError); + try { + remotingService.stop(criticalIOError); + } + catch (Throwable t) { + ActiveMQServerLogger.LOGGER.errorStoppingComponent(t, remotingService.getClass().getName()); + } // Stop the management service after the remoting service to ensure all acceptors are deregistered with JMX if (managementService != null) - managementService.unregisterServer(); + try { + managementService.unregisterServer(); + } + catch (Throwable t) { + ActiveMQServerLogger.LOGGER.errorStoppingComponent(t, managementService.getClass().getName()); + } + stopComponent(managementService); - stopComponent(resourceManager); - stopComponent(postOffice); if (scheduledPool != null && !scheduledPoolSupplied) { @@ -667,7 +693,7 @@ public class ActiveMQServerImpl implements ActiveMQServer { } } catch (InterruptedException e) { - // Ignore + ActiveMQServerLogger.LOGGER.interruptWhilstStoppingComponent(threadPool.getClass().getName()); } } @@ -676,8 +702,15 @@ public class ActiveMQServerImpl implements ActiveMQServer { if (!scheduledPoolSupplied) scheduledPool = null; - if (securityStore != null) - securityStore.stop(); + if (securityStore != null) { + try { + securityStore.stop(); + } + catch (Throwable t) { + ActiveMQServerLogger.LOGGER.errorStoppingComponent(t, managementService.getClass().getName()); + } + } + pagingManager = null; securityStore = null; @@ -699,11 +732,22 @@ public class ActiveMQServerImpl implements ActiveMQServer { // to display in the log message SimpleString tempNodeID = getNodeID(); if (activation != null) { - activation.close(failoverOnServerShutdown, restarting); + try { + activation.close(failoverOnServerShutdown, restarting); + } + catch (Throwable t) { + ActiveMQServerLogger.LOGGER.errorStoppingComponent(t, activation.getClass().getName()); + } } - if (backupActivationThread != null) { - backupActivationThread.join(30000); + if (backupActivationThread != null) { + try { + backupActivationThread.join(30000); + } + catch (InterruptedException e) { + ActiveMQServerLogger.LOGGER.interruptWhilstStoppingComponent(backupActivationThread.getClass().getName()); + } + if (backupActivationThread.isAlive()) { ActiveMQServerLogger.LOGGER.backupActivationDidntFinish(this); backupActivationThread.interrupt(); @@ -790,9 +834,15 @@ public class ActiveMQServerImpl implements ActiveMQServer { } - static void stopComponent(ActiveMQComponent component) throws Exception { - if (component != null) - component.stop(); + static void stopComponent(ActiveMQComponent component) { + try { + if (component != null) { + component.stop(); + } + } + catch (Throwable t) { + ActiveMQServerLogger.LOGGER.errorStoppingComponent(t, component.getClass().getName()); + } } // ActiveMQServer implementation