fix wrong ClassLoader for Delayed-Restart Thread, which has caused an ClassLoader leak.

Also added system properties to configure shutdown only, wait between stop and start and possibility to disable gc.
This commit is contained in:
Sebastian Sdorra
2019-11-21 16:20:55 +01:00
parent 755b99f524
commit d1a5f6f24b
6 changed files with 97 additions and 25 deletions

View File

@@ -90,8 +90,12 @@ public class LegmanScmEventBus extends ScmEventBus
@Override @Override
public void post(Object event) public void post(Object event)
{ {
logger.debug("post {} to event bus {}", event, name); if (eventBus != null) {
eventBus.post(event); logger.debug("post {} to event bus {}", event, name);
eventBus.post(event);
} else {
logger.error("failed to post event {}, because event bus is shutdown", event);
}
} }
/** /**
@@ -104,9 +108,12 @@ public class LegmanScmEventBus extends ScmEventBus
@Override @Override
public void register(Object object) public void register(Object object)
{ {
logger.trace("register {} to event bus {}", object, name); if (eventBus != null) {
eventBus.register(object); logger.trace("register {} to event bus {}", object, name);
eventBus.register(object);
} else {
logger.error("failed to register {}, because eventbus is shutdown", object);
}
} }
/** /**
@@ -118,22 +125,37 @@ public class LegmanScmEventBus extends ScmEventBus
@Override @Override
public void unregister(Object object) public void unregister(Object object)
{ {
logger.trace("unregister {} from event bus {}", object, name); if (eventBus != null) {
logger.trace("unregister {} from event bus {}", object, name);
try try {
{ eventBus.unregister(object);
eventBus.unregister(object); } catch (IllegalArgumentException ex) {
logger.trace("object {} was not registered", object);
}
} else {
logger.error("failed to unregister object {}, because event bus is shutdown", object);
} }
catch (IllegalArgumentException ex) }
{
logger.trace("object {} was not registered", object); @Subscribe(async = false)
public void shutdownEventBus(ShutdownEventBusEvent shutdownEventBusEvent) {
if (eventBus != null) {
logger.info("shutdown event bus executor for {}, because of received ShutdownEventBusEvent", name);
eventBus.shutdown();
eventBus = null;
} else {
logger.warn("event bus was already shutdown");
} }
} }
@Subscribe(async = false) @Subscribe(async = false)
public void recreateEventBus(RecreateEventBusEvent recreateEventBusEvent) { public void recreateEventBus(RecreateEventBusEvent recreateEventBusEvent) {
logger.info("shutdown event bus executor for {}", name); if (eventBus != null) {
eventBus.shutdown(); logger.info("shutdown event bus executor for {}, because of received RecreateEventBusEvent", name);
eventBus.shutdown();
}
logger.info("recreate event bus because of received RecreateEventBusEvent");
eventBus = create(); eventBus = create();
} }

View File

@@ -0,0 +1,4 @@
package sonia.scm.event;
public class ShutdownEventBusEvent {
}

View File

@@ -58,13 +58,16 @@ public class BootstrapContextFilter extends GuiceFilter {
private final BootstrapContextListener listener = new BootstrapContextListener(); private final BootstrapContextListener listener = new BootstrapContextListener();
private ClassLoader webAppClassLoader;
/** Field description */ /** Field description */
private FilterConfig filterConfig; private FilterConfig filterConfig;
@Override @Override
public void init(FilterConfig filterConfig) throws ServletException { public void init(FilterConfig filterConfig) throws ServletException {
this.filterConfig = filterConfig; this.filterConfig = filterConfig;
// store webapp classloader for delayed restarts
webAppClassLoader = Thread.currentThread().getContextClassLoader();
initializeContext(); initializeContext();
} }
@@ -97,7 +100,7 @@ public class BootstrapContextFilter extends GuiceFilter {
if (filterConfig == null) { if (filterConfig == null) {
LOG.error("filter config is null, scm-manager is not initialized"); LOG.error("filter config is null, scm-manager is not initialized");
} else { } else {
RestartStrategy restartStrategy = RestartStrategy.get(); RestartStrategy restartStrategy = RestartStrategy.get(webAppClassLoader);
restartStrategy.restart(new GuiceInjectionContext()); restartStrategy.restart(new GuiceInjectionContext());
} }
} }

View File

@@ -5,6 +5,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import sonia.scm.event.RecreateEventBusEvent; import sonia.scm.event.RecreateEventBusEvent;
import sonia.scm.event.ScmEventBus; import sonia.scm.event.ScmEventBus;
import sonia.scm.event.ShutdownEventBusEvent;
import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicLong;
@@ -13,20 +14,47 @@ import java.util.concurrent.atomic.AtomicLong;
*/ */
public class InjectionContextRestartStrategy implements RestartStrategy { public class InjectionContextRestartStrategy implements RestartStrategy {
private static final String DISABLE_RESTART_PROPERTY = "sonia.scm.restart.disable";
private static final String WAIT_PROPERTY = "sonia.scm.restart.wait";
private static final String DISABLE_GC_PROPERTY = "sonia.scm.restart.disable-gc";
private static final AtomicLong INSTANCE_COUNTER = new AtomicLong(); private static final AtomicLong INSTANCE_COUNTER = new AtomicLong();
private static final Logger LOG = LoggerFactory.getLogger(InjectionContextRestartStrategy.class); private static final Logger LOG = LoggerFactory.getLogger(InjectionContextRestartStrategy.class);
private long waitInMs = 250L; private boolean restartEnabled = !Boolean.getBoolean(DISABLE_RESTART_PROPERTY);
private long waitInMs = Integer.getInteger(WAIT_PROPERTY, 250);
private boolean gcEnabled = !Boolean.getBoolean(DISABLE_GC_PROPERTY);
private final ClassLoader webAppClassLoader;
InjectionContextRestartStrategy(ClassLoader webAppClassLoader) {
this.webAppClassLoader = webAppClassLoader;
}
@VisibleForTesting @VisibleForTesting
void setWaitInMs(long waitInMs) { void setWaitInMs(long waitInMs) {
this.waitInMs = waitInMs; this.waitInMs = waitInMs;
} }
@VisibleForTesting
void setGcEnabled(boolean gcEnabled) {
this.gcEnabled = gcEnabled;
}
@Override @Override
public void restart(InjectionContext context) { public void restart(InjectionContext context) {
LOG.warn("destroy injection context"); stop(context);
context.destroy(); if (restartEnabled) {
start(context);
} else {
LOG.warn("restarting context is disabled");
}
}
@SuppressWarnings("squid:S1215") // suppress explicit gc call warning
private void start(InjectionContext context) {
LOG.debug("use WebAppClassLoader as ContextClassLoader, to avoid ClassLoader leaks");
Thread.currentThread().setContextClassLoader(webAppClassLoader);
LOG.warn("send recreate eventbus event"); LOG.warn("send recreate eventbus event");
ScmEventBus.getInstance().post(new RecreateEventBusEvent()); ScmEventBus.getInstance().post(new RecreateEventBusEvent());
@@ -34,6 +62,12 @@ public class InjectionContextRestartStrategy implements RestartStrategy {
// restart context delayed, to avoid timing problems // restart context delayed, to avoid timing problems
new Thread(() -> { new Thread(() -> {
try { try {
if (gcEnabled){
LOG.info("call gc to clean up memory from old instances");
System.gc();
}
LOG.info("wait {}ms before re starting the context", waitInMs);
Thread.sleep(waitInMs); Thread.sleep(waitInMs);
LOG.warn("reinitialize injection context"); LOG.warn("reinitialize injection context");
@@ -45,6 +79,15 @@ public class InjectionContextRestartStrategy implements RestartStrategy {
LOG.error("failed to restart", ex); LOG.error("failed to restart", ex);
} }
}, "Delayed-Restart-" + INSTANCE_COUNTER.incrementAndGet()).start(); }, "Delayed-Restart-" + INSTANCE_COUNTER.incrementAndGet()).start();
}
private void stop(InjectionContext context) {
LOG.warn("destroy injection context");
context.destroy();
if (!restartEnabled) {
// shutdown eventbus, but do this only if restart is disabled
ScmEventBus.getInstance().post(new ShutdownEventBusEvent());
}
} }
} }

View File

@@ -13,7 +13,6 @@ public interface RestartStrategy {
* Initialize the injection context. * Initialize the injection context.
*/ */
void initialize(); void initialize();
/** /**
* Destroys the injection context. * Destroys the injection context.
*/ */
@@ -31,8 +30,8 @@ public interface RestartStrategy {
* *
* @return configured strategy * @return configured strategy
*/ */
static RestartStrategy get() { static RestartStrategy get(ClassLoader webAppClassLoader) {
return new InjectionContextRestartStrategy(); return new InjectionContextRestartStrategy(webAppClassLoader);
} }
} }

View File

@@ -18,11 +18,13 @@ class InjectionContextRestartStrategyTest {
@Mock @Mock
private RestartStrategy.InjectionContext context; private RestartStrategy.InjectionContext context;
private InjectionContextRestartStrategy strategy = new InjectionContextRestartStrategy(); private InjectionContextRestartStrategy strategy = new InjectionContextRestartStrategy(Thread.currentThread().getContextClassLoader());
@BeforeEach @BeforeEach
void setWaitToZero() { void setWaitToZero() {
strategy.setWaitInMs(0L); strategy.setWaitInMs(0L);
// disable gc during tests
strategy.setGcEnabled(false);
} }
@Test @Test
@@ -47,7 +49,6 @@ class InjectionContextRestartStrategyTest {
@Test @Test
void shouldRegisterContextAfterRestart() throws InterruptedException { void shouldRegisterContextAfterRestart() throws InterruptedException {
TestingInjectionContext ctx = new TestingInjectionContext(); TestingInjectionContext ctx = new TestingInjectionContext();
strategy.restart(ctx); strategy.restart(ctx);
Thread.sleep(50L); Thread.sleep(50L);