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.
This commit is contained in:
Howard Gao 2017-02-23 12:25:02 +08:00
parent d01874ae82
commit e09be4864a
7 changed files with 118 additions and 17 deletions

View File

@ -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();
}

View File

@ -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();
}

View File

@ -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;
}

View File

@ -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.

View File

@ -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
@ -667,19 +668,24 @@ public class ActiveMQServerImpl implements ActiveMQServer {
thread.start();
}
@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());
}

View File

@ -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;
}
}
}

View File

@ -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();