activemq-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From clebertsuco...@apache.org
Subject [1/2] activemq-artemis git commit: ARTEMIS-988 Regression: web tmp dir not cleaned up
Date Thu, 23 Feb 2017 16:11:51 GMT
Repository: activemq-artemis
Updated Branches:
  refs/heads/master d01874ae8 -> fbf3a726c


ARTEMIS-988 Regression: web tmp dir not cleaned up

Due to recent changes, the web component is shutdown by the
server, but the shutdown flag is lost so the web component's
cleanup check method is not get called and the web's tmp
dir is left there after user stopped the broker (control-c).

The fix is add a suitable API to allow passing of the
flag so the web component can make sure its tmp dir gets
cleaned up properly before exiting the VM.


Project: http://git-wip-us.apache.org/repos/asf/activemq-artemis/repo
Commit: http://git-wip-us.apache.org/repos/asf/activemq-artemis/commit/e09be486
Tree: http://git-wip-us.apache.org/repos/asf/activemq-artemis/tree/e09be486
Diff: http://git-wip-us.apache.org/repos/asf/activemq-artemis/diff/e09be486

Branch: refs/heads/master
Commit: e09be4864aff04db372fb4c72c0d01c2fe91e798
Parents: d01874a
Author: Howard Gao <howard.gao@gmail.com>
Authored: Thu Feb 23 12:25:02 2017 +0800
Committer: Howard Gao <howard.gao@gmail.com>
Committed: Thu Feb 23 12:39:50 2017 +0800

----------------------------------------------------------------------
 .../activemq/artemis/cli/commands/Run.java      |  4 +-
 .../artemis/integration/FileBroker.java         | 10 ++-
 .../artemis/core/server/ServiceComponent.java   |  3 +-
 .../artemis/core/server/ActiveMQServer.java     |  2 +-
 .../core/server/impl/ActiveMQServerImpl.java    | 36 +++++++---
 .../core/server/impl/EmbeddedServerTest.java    | 75 +++++++++++++++++++-
 .../artemis/component/WebServerComponent.java   |  5 ++
 7 files changed, 118 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/e09be486/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Run.java
----------------------------------------------------------------------
diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Run.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Run.java
index 5efbef4..1811365 100644
--- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Run.java
+++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Run.java
@@ -134,7 +134,7 @@ public class Run extends LockAbstract {
             if (file.exists()) {
                try {
                   try {
-                     server.stop(true);
+                     server.exit();
                   } catch (Exception e) {
                      e.printStackTrace();
                   }
@@ -155,7 +155,7 @@ public class Run extends LockAbstract {
          @Override
          public void run() {
             try {
-               server.stop(true);
+               server.exit();
             } catch (Exception e) {
                e.printStackTrace();
             }

http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/e09be486/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/FileBroker.java
----------------------------------------------------------------------
diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/FileBroker.java
b/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/FileBroker.java
index 2776769..03df935 100644
--- a/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/FileBroker.java
+++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/FileBroker.java
@@ -118,15 +118,19 @@ public class FileBroker implements Broker {
    }
 
    @Override
-   public void stop(boolean isShutdown) throws Exception {
+   public void exit() throws Exception {
+      stop(true);
+   }
+
+   private void stop(boolean isShutdown) throws Exception {
       if (!started) {
          return;
       }
       ActiveMQComponent[] mqComponents = new ActiveMQComponent[components.size()];
       components.values().toArray(mqComponents);
       for (int i = mqComponents.length - 1; i >= 0; i--) {
-         if (mqComponents[i] instanceof ServiceComponent) {
-            ((ServiceComponent)mqComponents[i]).stop(isShutdown);
+         if (mqComponents[i] instanceof ServiceComponent && isShutdown) {
+            ((ServiceComponent) mqComponents[i]).exit();
          } else {
             mqComponents[i].stop();
          }

http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/e09be486/artemis-commons/src/main/java/org/apache/activemq/artemis/core/server/ServiceComponent.java
----------------------------------------------------------------------
diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/core/server/ServiceComponent.java
b/artemis-commons/src/main/java/org/apache/activemq/artemis/core/server/ServiceComponent.java
index 2fb0b6f..6c347dd 100644
--- a/artemis-commons/src/main/java/org/apache/activemq/artemis/core/server/ServiceComponent.java
+++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/core/server/ServiceComponent.java
@@ -21,5 +21,6 @@ package org.apache.activemq.artemis.core.server;
  */
 public interface ServiceComponent extends ActiveMQComponent {
 
-   void stop(boolean isShutdown) throws Exception;
+   //called by shutdown hooks before exit the VM
+   void exit() throws Exception;
 }

http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/e09be486/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java
----------------------------------------------------------------------
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java
b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java
index bc02010..5798142 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java
@@ -64,7 +64,7 @@ import org.apache.activemq.artemis.utils.ExecutorFactory;
  * <p>
  * This is not part of our public API.
  */
-public interface ActiveMQServer extends ActiveMQComponent {
+public interface ActiveMQServer extends ServiceComponent {
 
    /**
     * Sets the server identity.

http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/e09be486/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
----------------------------------------------------------------------
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 0332137..aa1ebf3 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
@@ -130,6 +130,7 @@ import org.apache.activemq.artemis.core.server.QueueQueryResult;
 import org.apache.activemq.artemis.api.core.RoutingType;
 import org.apache.activemq.artemis.core.server.SecuritySettingPlugin;
 import org.apache.activemq.artemis.core.server.ServerSession;
+import org.apache.activemq.artemis.core.server.ServiceComponent;
 import org.apache.activemq.artemis.core.server.ServiceRegistry;
 import org.apache.activemq.artemis.core.server.cluster.BackupManager;
 import org.apache.activemq.artemis.core.server.cluster.ClusterManager;
@@ -335,7 +336,7 @@ public class ActiveMQServerImpl implements ActiveMQServer {
 
       @Override
       public void stop() throws Exception {
-         internalStop();
+         internalStop(false);
       }
 
       @Override
@@ -668,18 +669,23 @@ public class ActiveMQServerImpl implements ActiveMQServer {
    }
 
    @Override
+   public void exit() throws Exception {
+      internalStop(true);
+   }
+
+   @Override
    public final void stop() throws Exception {
+      internalStop(false);
+   }
+
+   private void internalStop(boolean isExit) throws Exception {
       try {
-         internalStop();
+         stop(false, isExit);
       } finally {
          networkHealthCheck.stop();
       }
    }
 
-   private void internalStop() throws Exception {
-      stop(false);
-   }
-
    @Override
    public void addActivationParam(String key, Object val) {
       activationParams.put(key, val);
@@ -810,7 +816,11 @@ public class ActiveMQServerImpl implements ActiveMQServer {
 
    @Override
    public final void stop(boolean failoverOnServerShutdown) throws Exception {
-      stop(failoverOnServerShutdown, false, false);
+      stop(failoverOnServerShutdown, false, false, false);
+   }
+
+   public final void stop(boolean failoverOnServerShutdown, boolean isExit) throws Exception
{
+      stop(failoverOnServerShutdown, false, false, isExit);
    }
 
    @Override
@@ -830,12 +840,16 @@ public class ActiveMQServerImpl implements ActiveMQServer {
       }
    }
 
+   void stop(boolean failoverOnServerShutdown, final boolean criticalIOError, boolean restarting)
{
+      this.stop(failoverOnServerShutdown, criticalIOError, restarting, false);
+   }
+
    /**
     * Stops the server
     *
     * @param criticalIOError whether we have encountered an IO error with the journal etc
     */
-   void stop(boolean failoverOnServerShutdown, final boolean criticalIOError, boolean restarting)
{
+   void stop(boolean failoverOnServerShutdown, final boolean criticalIOError, boolean restarting,
boolean isExit) {
 
       synchronized (this) {
          if (state == SERVER_STATE.STOPPED || state == SERVER_STATE.STOPPING) {
@@ -1024,7 +1038,11 @@ public class ActiveMQServerImpl implements ActiveMQServer {
 
       for (ActiveMQComponent externalComponent : externalComponents) {
          try {
-            externalComponent.stop();
+            if (isExit && externalComponent instanceof ServiceComponent) {
+               ((ServiceComponent)externalComponent).exit();
+            } else {
+               externalComponent.stop();
+            }
          } catch (Exception e) {
             ActiveMQServerLogger.LOGGER.errorStoppingComponent(e, externalComponent.getClass().getName());
          }

http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/e09be486/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/EmbeddedServerTest.java
----------------------------------------------------------------------
diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/EmbeddedServerTest.java
b/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/EmbeddedServerTest.java
index 4b882cf..e47440d 100644
--- a/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/EmbeddedServerTest.java
+++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/EmbeddedServerTest.java
@@ -24,13 +24,18 @@ import org.apache.activemq.artemis.api.core.TransportConfiguration;
 import org.apache.activemq.artemis.core.config.Configuration;
 import org.apache.activemq.artemis.core.config.impl.ConfigurationImpl;
 import org.apache.activemq.artemis.core.remoting.impl.invm.InVMAcceptorFactory;
+import org.apache.activemq.artemis.core.server.ActiveMQComponent;
 import org.apache.activemq.artemis.core.server.ActiveMQServer;
 import org.apache.activemq.artemis.core.server.ActiveMQServers;
+import org.apache.activemq.artemis.core.server.ServiceComponent;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
 public class EmbeddedServerTest {
 
    private static final String SERVER_LOCK_NAME = "server.lock";
@@ -64,6 +69,74 @@ public class EmbeddedServerTest {
    public void testNoLockFileWithPersistenceFalse() {
       Path journalDir = Paths.get(SERVER_JOURNAL_DIR, SERVER_LOCK_NAME);
       boolean lockExists = Files.exists(journalDir);
-      Assert.assertFalse(lockExists);
+      assertFalse(lockExists);
+   }
+
+   @Test
+   //make sure the correct stop/exit API is called.
+   public void testExternalComponentStop() throws Exception {
+      FakeExternalComponent normalComponent = new FakeExternalComponent();
+      FakeExternalServiceComponent serviceComponent = new FakeExternalServiceComponent();
+
+      server.addExternalComponent(normalComponent);
+      server.addExternalComponent(serviceComponent);
+
+      server.stop();
+      assertTrue(normalComponent.stopCalled);
+
+      assertTrue(serviceComponent.stopCalled);
+      assertFalse(serviceComponent.exitCalled);
+
+      normalComponent.resetFlags();
+      serviceComponent.resetFlags();
+
+      server.start();
+      server.exit();
+      assertTrue(normalComponent.stopCalled);
+
+      assertFalse(serviceComponent.stopCalled);
+      assertTrue(serviceComponent.exitCalled);
+   }
+
+   private class FakeExternalComponent implements ActiveMQComponent {
+
+      volatile boolean startCalled;
+      volatile boolean stopCalled;
+
+      @Override
+      public void start() throws Exception {
+         startCalled = true;
+      }
+
+      @Override
+      public void stop() throws Exception {
+         stopCalled = true;
+      }
+
+      @Override
+      public boolean isStarted() {
+         return startCalled;
+      }
+
+      public void resetFlags() {
+         startCalled = false;
+         stopCalled = false;
+      }
+   }
+
+   private class FakeExternalServiceComponent extends FakeExternalComponent implements ServiceComponent
{
+
+      volatile boolean exitCalled;
+
+      @Override
+      public void exit() throws Exception {
+         exitCalled = true;
+      }
+
+      @Override
+      public void resetFlags() {
+         super.resetFlags();
+         exitCalled = false;
+      }
    }
 }

http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/e09be486/artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java
----------------------------------------------------------------------
diff --git a/artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java
b/artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java
index a6df272..6dce708 100644
--- a/artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java
+++ b/artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java
@@ -150,6 +150,7 @@ public class WebServerComponent implements ExternalComponent {
                //somehow when broker is stopped and restarted quickly
                //this tmpdir won't get deleted sometimes
                boolean fileDeleted = TimeUtils.waitOnBoolean(false, 5000, tmpdir::exists);
+
                if (!fileDeleted) {
                   //because the execution order of shutdown hooks are
                   //not determined, so it's possible that the deleteOnExit
@@ -190,6 +191,10 @@ public class WebServerComponent implements ExternalComponent {
    }
 
    @Override
+   public void exit() throws Exception {
+      stop(true);
+   }
+
    public void stop(boolean isShutdown) throws Exception {
       if (isShutdown) {
          internalStop();


Mime
View raw message