From 5ef5c895b63b6e4f506b179be75d14d8c2a70bd7 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 5 Jun 2019 16:03:51 +0200 Subject: [PATCH 01/16] update legman to version 1.5.0 The version 1.5.0 fixes a classloader leak, because all EventBus instances are sharing the same cache in versions before 1.5.0. --- pom.xml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index d8437f2934..75c6c3d727 100644 --- a/pom.xml +++ b/pom.xml @@ -819,7 +819,7 @@ 2.3.0 - 1.4.2 + 1.5.0 9.4.14.v20181114 @@ -835,7 +835,6 @@ 26.0-jre - 2.2.3 8.11.4 From 1d7f982951dc3b8360e1b886f611015938d3fd13 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 5 Jun 2019 16:05:01 +0200 Subject: [PATCH 02/16] remove EventBus registration, because there is no subscribe --- .../sonia/scm/boot/BootstrapContextListener.java | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextListener.java b/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextListener.java index 3af8a76650..6ca537673a 100644 --- a/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextListener.java +++ b/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextListener.java @@ -44,8 +44,6 @@ import sonia.scm.SCMContext; import sonia.scm.ScmContextListener; import sonia.scm.ScmEventBusModule; import sonia.scm.ScmInitializerModule; -import sonia.scm.Stage; -import sonia.scm.event.ScmEventBus; import sonia.scm.plugin.DefaultPluginLoader; import sonia.scm.plugin.Plugin; import sonia.scm.plugin.PluginException; @@ -141,13 +139,6 @@ public class BootstrapContextListener implements ServletContextListener { createContextListener(pluginDirectory); contextListener.contextInitialized(sce); - - // register for restart events - if (!registered && (SCMContext.getContext().getStage() == Stage.DEVELOPMENT)) { - logger.info("register for restart events"); - ScmEventBus.getInstance().register(this); - registered = true; - } } private void createContextListener(File pluginDirectory) { @@ -404,9 +395,6 @@ public class BootstrapContextListener implements ServletContextListener { /** Field description */ private ScmContextListener contextListener; - /** Field description */ - private boolean registered = false; - private static class ScmContextListenerModule extends AbstractModule { @Override protected void configure() { From 73dc0d0544a8d74bc87b8731ef078920d31aa722 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 5 Jun 2019 16:09:01 +0200 Subject: [PATCH 03/16] clear ResteasyProviderFactory and RuntimeDelegate during restart This change is required, because some cachings of resteasy are not cleared by removing them from ServletContext. Some classes use static or ThreadLocals, which are causing a ClassLoader leak. However this change will clear those caches. --- .../src/main/java/sonia/scm/boot/ServletContextCleaner.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scm-webapp/src/main/java/sonia/scm/boot/ServletContextCleaner.java b/scm-webapp/src/main/java/sonia/scm/boot/ServletContextCleaner.java index 8b152ce329..085b752096 100644 --- a/scm-webapp/src/main/java/sonia/scm/boot/ServletContextCleaner.java +++ b/scm-webapp/src/main/java/sonia/scm/boot/ServletContextCleaner.java @@ -1,10 +1,12 @@ package sonia.scm.boot; import com.google.common.collect.ImmutableSet; +import org.jboss.resteasy.spi.ResteasyProviderFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.servlet.ServletContext; +import javax.ws.rs.ext.RuntimeDelegate; import java.util.Enumeration; import java.util.Set; @@ -46,6 +48,10 @@ final class ServletContextCleaner { LOG.info("keep attribute {} in servlet context", name); } } + + ResteasyProviderFactory.clearInstanceIfEqual(ResteasyProviderFactory.getInstance()); + ResteasyProviderFactory.clearContextData(); + RuntimeDelegate.setInstance(null); } private static boolean shouldRemove(String name) { From 2b64a49e11464a46cc579a7d0bae13396b1cf024 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 5 Jun 2019 16:10:15 +0200 Subject: [PATCH 04/16] recreate the EventBus on restart to avoid classloader leaks --- .../scm/boot/BootstrapContextFilter.java | 7 ++++++- .../sonia/scm/event/LegmanScmEventBus.java | 21 ++++++++++++++----- .../scm/event/RecreateEventBusEvent.java | 7 +++++++ 3 files changed, 29 insertions(+), 6 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/event/RecreateEventBusEvent.java diff --git a/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextFilter.java b/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextFilter.java index aec8e2d653..9c0648e22f 100644 --- a/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextFilter.java +++ b/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextFilter.java @@ -39,6 +39,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.SCMContext; import sonia.scm.Stage; +import sonia.scm.event.RecreateEventBusEvent; import sonia.scm.event.ScmEventBus; import javax.servlet.FilterConfig; @@ -72,7 +73,7 @@ public class BootstrapContextFilter extends GuiceFilter * * @throws ServletException */ - @Subscribe(async = false) + @Subscribe public void handleRestartEvent(RestartEvent event) throws ServletException { logger.warn("received restart event from {} with reason: {}", @@ -87,6 +88,10 @@ public class BootstrapContextFilter extends GuiceFilter logger.warn("destroy filter pipeline, because of a received restart event"); destroy(); + logger.warn("send recreate eventbus event"); + ScmEventBus.getInstance().post(new RecreateEventBusEvent()); + ScmEventBus.getInstance().register(this); + logger.warn("reinitialize filter pipeline, because of a received restart event"); initGuice(); } diff --git a/scm-webapp/src/main/java/sonia/scm/event/LegmanScmEventBus.java b/scm-webapp/src/main/java/sonia/scm/event/LegmanScmEventBus.java index b491c820ea..fb8fe6d2de 100644 --- a/scm-webapp/src/main/java/sonia/scm/event/LegmanScmEventBus.java +++ b/scm-webapp/src/main/java/sonia/scm/event/LegmanScmEventBus.java @@ -36,7 +36,7 @@ package sonia.scm.event; //~--- non-JDK imports -------------------------------------------------------- import com.github.legman.EventBus; - +import com.github.legman.Subscribe; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -62,9 +62,13 @@ public class LegmanScmEventBus extends ScmEventBus * Constructs ... * */ - public LegmanScmEventBus() - { - eventBus = new EventBus(NAME); + public LegmanScmEventBus() { + eventBus = create(); + } + + private EventBus create() { + logger.info("create new event bus {}", NAME); + return new EventBus(NAME); } //~--- methods -------------------------------------------------------------- @@ -118,8 +122,15 @@ public class LegmanScmEventBus extends ScmEventBus } } + @Subscribe(async = false) + public void recreateEventBus(RecreateEventBusEvent recreateEventBusEvent) { + logger.info("shutdown event bus executor"); + eventBus.shutdown(); + eventBus = create(); + } + //~--- fields --------------------------------------------------------------- /** event bus */ - private final EventBus eventBus; + private EventBus eventBus; } diff --git a/scm-webapp/src/main/java/sonia/scm/event/RecreateEventBusEvent.java b/scm-webapp/src/main/java/sonia/scm/event/RecreateEventBusEvent.java new file mode 100644 index 0000000000..55030648ba --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/event/RecreateEventBusEvent.java @@ -0,0 +1,7 @@ +package sonia.scm.event; + +/** + * This event forces the {@link ScmEventBus} to recreate the underlying implementation and to clear all its caches. + * Note: After this event is fired, every subscription is removed from the event bus. + */ +public final class RecreateEventBusEvent {} From 3c373a4c4db2a83678bd742b8b4a4ed5b93bd3a2 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 5 Jun 2019 16:15:06 +0200 Subject: [PATCH 05/16] replace QuartzScheduler with a more lightweight scheduler The new scheduler is based on the cron-utils package and uses the same cron syntax as quartz. CronScheduler was mainly introduced, because of a ClassLoader leak with the old Quartz implementation. The leak comes from shiros use of InheriatableThreadLocal in combination with the WorkerThreads of Quartz. CronScheduler uses a ThreadFactory which clears the Shiro context before a new Thread is created (see CronThreadFactory). --- scm-webapp/pom.xml | 12 +- .../main/java/sonia/scm/ScmServletModule.java | 4 +- .../sonia/scm/schedule/CronExpression.java | 59 +++++ .../sonia/scm/schedule/CronScheduler.java | 57 +++++ .../java/sonia/scm/schedule/CronTask.java | 74 ++++++ .../sonia/scm/schedule/CronTaskFactory.java | 32 +++ .../sonia/scm/schedule/CronThreadFactory.java | 47 ++++ .../scm/schedule/InjectionEnabledJob.java | 85 ------- .../schedule/PrivilegedRunnableFactory.java | 29 +++ .../sonia/scm/schedule/QuartzScheduler.java | 175 -------------- .../java/sonia/scm/schedule/QuartzTask.java | 68 ------ .../scm/schedule/CronExpressionTest.java | 75 ++++++ .../sonia/scm/schedule/CronSchedulerTest.java | 66 ++++++ .../scm/schedule/CronTaskFactoryTest.java | 67 ++++++ .../java/sonia/scm/schedule/CronTaskTest.java | 95 ++++++++ .../scm/schedule/CronThreadFactoryTest.java | 87 +++++++ .../scm/schedule/InjectionEnabledJobTest.java | 168 ------------- .../PrivilegedRunnableFactoryTest.java | 53 +++++ .../scm/schedule/QuartzSchedulerTest.java | 222 ------------------ .../sonia/scm/schedule/QuartzTaskTest.java | 89 ------- 20 files changed, 746 insertions(+), 818 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/schedule/CronExpression.java create mode 100644 scm-webapp/src/main/java/sonia/scm/schedule/CronScheduler.java create mode 100644 scm-webapp/src/main/java/sonia/scm/schedule/CronTask.java create mode 100644 scm-webapp/src/main/java/sonia/scm/schedule/CronTaskFactory.java create mode 100644 scm-webapp/src/main/java/sonia/scm/schedule/CronThreadFactory.java delete mode 100644 scm-webapp/src/main/java/sonia/scm/schedule/InjectionEnabledJob.java create mode 100644 scm-webapp/src/main/java/sonia/scm/schedule/PrivilegedRunnableFactory.java delete mode 100644 scm-webapp/src/main/java/sonia/scm/schedule/QuartzScheduler.java delete mode 100644 scm-webapp/src/main/java/sonia/scm/schedule/QuartzTask.java create mode 100644 scm-webapp/src/test/java/sonia/scm/schedule/CronExpressionTest.java create mode 100644 scm-webapp/src/test/java/sonia/scm/schedule/CronSchedulerTest.java create mode 100644 scm-webapp/src/test/java/sonia/scm/schedule/CronTaskFactoryTest.java create mode 100644 scm-webapp/src/test/java/sonia/scm/schedule/CronTaskTest.java create mode 100644 scm-webapp/src/test/java/sonia/scm/schedule/CronThreadFactoryTest.java delete mode 100644 scm-webapp/src/test/java/sonia/scm/schedule/InjectionEnabledJobTest.java create mode 100644 scm-webapp/src/test/java/sonia/scm/schedule/PrivilegedRunnableFactoryTest.java delete mode 100644 scm-webapp/src/test/java/sonia/scm/schedule/QuartzSchedulerTest.java delete mode 100644 scm-webapp/src/test/java/sonia/scm/schedule/QuartzTaskTest.java diff --git a/scm-webapp/pom.xml b/scm-webapp/pom.xml index f3c96af5fc..00b25c2273 100644 --- a/scm-webapp/pom.xml +++ b/scm-webapp/pom.xml @@ -245,15 +245,9 @@ - org.quartz-scheduler - quartz - ${quartz.version} - - - c3p0 - c3p0 - - + com.cronutils + cron-utils + 8.1.1 diff --git a/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java b/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java index 83eb03fe61..49ac0a3d59 100644 --- a/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java +++ b/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java @@ -80,7 +80,7 @@ import sonia.scm.repository.api.RepositoryServiceFactory; import sonia.scm.repository.spi.HookEventFacade; import sonia.scm.repository.xml.XmlRepositoryDAO; import sonia.scm.repository.xml.XmlRepositoryRoleDAO; -import sonia.scm.schedule.QuartzScheduler; +import sonia.scm.schedule.CronScheduler; import sonia.scm.schedule.Scheduler; import sonia.scm.security.AccessTokenCookieIssuer; import sonia.scm.security.AuthorizationChangedEventProducer; @@ -218,7 +218,7 @@ public class ScmServletModule extends ServletModule bind(PluginManager.class, DefaultPluginManager.class); // bind scheduler - bind(Scheduler.class).to(QuartzScheduler.class); + bind(Scheduler.class).to(CronScheduler.class); // bind health check stuff bind(HealthCheckContextListener.class); diff --git a/scm-webapp/src/main/java/sonia/scm/schedule/CronExpression.java b/scm-webapp/src/main/java/sonia/scm/schedule/CronExpression.java new file mode 100644 index 0000000000..fbbff51f57 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/schedule/CronExpression.java @@ -0,0 +1,59 @@ +package sonia.scm.schedule; + +import com.cronutils.model.Cron; +import com.cronutils.model.CronType; +import com.cronutils.model.definition.CronDefinition; +import com.cronutils.model.definition.CronDefinitionBuilder; +import com.cronutils.model.time.ExecutionTime; +import com.cronutils.parser.CronParser; + +import java.time.Clock; +import java.time.Duration; +import java.time.ZonedDateTime; +import java.util.Optional; + +final class CronExpression { + + private final Clock clock; + private final String expression; + private final ExecutionTime executionTime; + + CronExpression(String expression) { + this(Clock.systemUTC(), expression); + } + + CronExpression(Clock clock, String expression) { + this.clock = clock; + this.expression = expression; + executionTime = createExecutionTime(expression); + } + + boolean shouldRun(ZonedDateTime time) { + ZonedDateTime now = ZonedDateTime.now(clock); + return time.isBefore(now); + } + + Optional calculateNextRun() { + ZonedDateTime now = ZonedDateTime.now(clock); + Optional nextExecution = executionTime.nextExecution(now); + if (nextExecution.isPresent()) { + ZonedDateTime next = nextExecution.get(); + if (Duration.between(now, next).toMillis() < 1000) { + return executionTime.nextExecution(now.plusSeconds(1L)); + } + } + return nextExecution; + } + + private ExecutionTime createExecutionTime(String expression) { + CronDefinition cronDefinition = CronDefinitionBuilder.instanceDefinitionFor(CronType.QUARTZ); + CronParser parser = new CronParser(cronDefinition); + Cron cron = parser.parse(expression); + return ExecutionTime.forCron(cron); + } + + @Override + public String toString() { + return expression; + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/schedule/CronScheduler.java b/scm-webapp/src/main/java/sonia/scm/schedule/CronScheduler.java new file mode 100644 index 0000000000..7899697746 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/schedule/CronScheduler.java @@ -0,0 +1,57 @@ +package sonia.scm.schedule; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.inject.Inject; +import javax.inject.Singleton; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +@Singleton +public class CronScheduler implements Scheduler { + + private static final Logger LOG = LoggerFactory.getLogger(CronScheduler.class); + + private final ScheduledExecutorService executorService; + private final CronTaskFactory taskFactory; + + @Inject + public CronScheduler(CronTaskFactory taskFactory) { + this.taskFactory = taskFactory; + this.executorService = createExecutor(); + } + + private ScheduledExecutorService createExecutor() { + return Executors.newScheduledThreadPool(2, new CronThreadFactory()); + } + + @Override + public CronTask schedule(String expression, Runnable runnable) { + return schedule(taskFactory.create(expression, runnable)); + } + + @Override + public CronTask schedule(String expression, Class runnable) { + return schedule(taskFactory.create(expression, runnable)); + } + + private CronTask schedule(CronTask task) { + if (task.hasNextRun()) { + LOG.debug("schedule task {}", task); + Future future = executorService.scheduleAtFixedRate(task, 0L, 1L, TimeUnit.SECONDS); + task.setFuture(future); + } else { + LOG.debug("skip scheduling, because task {} has no next run", task); + } + return task; + } + + @Override + public void close() { + LOG.debug("shutdown underlying executor service"); + executorService.shutdown(); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/schedule/CronTask.java b/scm-webapp/src/main/java/sonia/scm/schedule/CronTask.java new file mode 100644 index 0000000000..0e8c636184 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/schedule/CronTask.java @@ -0,0 +1,74 @@ +package sonia.scm.schedule; + +import com.cronutils.utils.VisibleForTesting; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.time.ZonedDateTime; +import java.util.Optional; +import java.util.concurrent.Future; + +class CronTask implements Task, Runnable { + + private static final Logger LOG = LoggerFactory.getLogger(CronTask.class); + + private final String name; + private final CronExpression expression; + private final Runnable runnable; + + private ZonedDateTime nextRun; + private Future future; + + CronTask(String name, CronExpression expression, Runnable runnable) { + this.name = name; + this.expression = expression; + this.runnable = runnable; + this.nextRun = expression.calculateNextRun().orElse(null); + } + + void setFuture(Future future) { + this.future = future; + } + + @Override + public synchronized void run() { + if (expression.shouldRun(nextRun)) { + LOG.debug("execute task {}, because of matching expression {}", name, expression); + runnable.run(); + Optional next = expression.calculateNextRun(); + if (next.isPresent()) { + nextRun = next.get(); + } else { + LOG.debug("cancel task {}, because expression {} has no next execution", name, expression); + cancel(); + } + } else { + LOG.trace("skip execution of task {}, because expression {} does not match", name, expression); + } + } + + boolean hasNextRun() { + return nextRun != null; + } + + @VisibleForTesting + String getName() { + return name; + } + + @VisibleForTesting + CronExpression getExpression() { + return expression; + } + + @Override + public synchronized void cancel() { + LOG.debug("cancel task {} with expression {}", name, expression); + future.cancel(false); + } + + @Override + public String toString() { + return name + "(" + expression + ")"; + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/schedule/CronTaskFactory.java b/scm-webapp/src/main/java/sonia/scm/schedule/CronTaskFactory.java new file mode 100644 index 0000000000..6ad7096363 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/schedule/CronTaskFactory.java @@ -0,0 +1,32 @@ +package sonia.scm.schedule; + +import com.google.inject.Injector; +import com.google.inject.util.Providers; + +import javax.inject.Inject; +import javax.inject.Provider; + +class CronTaskFactory { + + private final Injector injector; + private final PrivilegedRunnableFactory runnableFactory; + + @Inject + public CronTaskFactory(Injector injector, PrivilegedRunnableFactory runnableFactory) { + this.injector = injector; + this.runnableFactory = runnableFactory; + } + + CronTask create(String expression, Runnable runnable) { + return create(expression, runnable.getClass().getName(), Providers.of(runnable)); + } + + CronTask create(String expression, Class runnable) { + return create(expression, runnable.getName(), injector.getProvider(runnable)); + } + + private CronTask create(String expression, String name, Provider runnableProvider) { + Runnable runnable = runnableFactory.create(runnableProvider); + return new CronTask(name, new CronExpression(expression), runnable); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/schedule/CronThreadFactory.java b/scm-webapp/src/main/java/sonia/scm/schedule/CronThreadFactory.java new file mode 100644 index 0000000000..f4f77d1c3e --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/schedule/CronThreadFactory.java @@ -0,0 +1,47 @@ +package sonia.scm.schedule; + +import org.apache.shiro.util.ThreadContext; + +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.atomic.AtomicLong; + +/** + * This thread factory creates threads without a shiro context. + * This is to avoid classloader leaks, because the {@link ThreadContext} of shiro uses {@link InheritableThreadLocal}, + * which could bind a class with a reference to a {@link sonia.scm.plugin.PluginClassLoader}. + */ +class CronThreadFactory implements ThreadFactory, AutoCloseable { + + private static final String NAME_TEMPLATE = "CronScheduler-%d-%d"; + + private static final AtomicLong FACTORY_COUNTER = new AtomicLong(); + + private final ExecutorService executorService = Executors.newSingleThreadExecutor(); + private final long factoryId = FACTORY_COUNTER.incrementAndGet(); + private final AtomicLong threadCounter = new AtomicLong(); + + @Override + public Thread newThread(final Runnable r) { + try { + return executorService.submit(() -> { + ThreadContext.remove(); + return new Thread(r, createName()); + }).get(); + } catch (InterruptedException | ExecutionException e) { + throw new RuntimeException("failed to schedule runnable"); + } + } + + private String createName() { + long threadId = threadCounter.incrementAndGet(); + return String.format(NAME_TEMPLATE, factoryId, threadId); + } + + @Override + public void close() { + executorService.shutdown(); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/schedule/InjectionEnabledJob.java b/scm-webapp/src/main/java/sonia/scm/schedule/InjectionEnabledJob.java deleted file mode 100644 index dd8312a6d8..0000000000 --- a/scm-webapp/src/main/java/sonia/scm/schedule/InjectionEnabledJob.java +++ /dev/null @@ -1,85 +0,0 @@ -/*** - * Copyright (c) 2015, Sebastian Sdorra - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * 3. Neither the name of SCM-Manager; nor the names of its - * contributors may be used to endorse or promote products derived from this - * software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE - * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY - * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES - * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON - * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS - * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - * https://bitbucket.org/sdorra/scm-manager - * - */ - -package sonia.scm.schedule; - -import com.google.common.base.Preconditions; -import com.google.inject.Injector; -import com.google.inject.Provider; -import org.quartz.Job; -import org.quartz.JobDataMap; -import org.quartz.JobDetail; -import org.quartz.JobExecutionContext; -import org.quartz.JobExecutionException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import sonia.scm.web.security.AdministrationContext; - -/** - * InjectionEnabledJob allows the execution of quartz jobs and enable injection on them. - * - * @author Sebastian Sdorra - * @since 1.47 - */ -public class InjectionEnabledJob implements Job { - - private static final Logger logger = LoggerFactory.getLogger(InjectionEnabledJob.class); - - @Override - @SuppressWarnings("unchecked") - public void execute(JobExecutionContext jec) throws JobExecutionException { - Preconditions.checkNotNull(jec, "execution context is null"); - - JobDetail detail = jec.getJobDetail(); - Preconditions.checkNotNull(detail, "job detail not provided"); - - JobDataMap dataMap = detail.getJobDataMap(); - Preconditions.checkNotNull(dataMap, "job detail does not contain data map"); - - Injector injector = (Injector) dataMap.get(Injector.class.getName()); - Preconditions.checkNotNull(injector, "data map does not contain injector"); - - final Provider runnableProvider = (Provider) dataMap.get(Runnable.class.getName()); - if (runnableProvider == null) { - throw new JobExecutionException("could not find runnable provider"); - } - - AdministrationContext ctx = injector.getInstance(AdministrationContext.class); - ctx.runAsAdmin(() -> { - logger.trace("create runnable from provider"); - Runnable runnable = runnableProvider.get(); - logger.debug("execute injection enabled job {}", runnable.getClass()); - runnable.run(); - }); - } - - -} diff --git a/scm-webapp/src/main/java/sonia/scm/schedule/PrivilegedRunnableFactory.java b/scm-webapp/src/main/java/sonia/scm/schedule/PrivilegedRunnableFactory.java new file mode 100644 index 0000000000..da630a78f2 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/schedule/PrivilegedRunnableFactory.java @@ -0,0 +1,29 @@ +package sonia.scm.schedule; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import sonia.scm.web.security.AdministrationContext; + +import javax.inject.Inject; +import javax.inject.Provider; + +class PrivilegedRunnableFactory { + + private static final Logger LOG = LoggerFactory.getLogger(PrivilegedRunnableFactory.class); + + private final AdministrationContext context; + + @Inject + PrivilegedRunnableFactory(AdministrationContext context) { + this.context = context; + } + + public Runnable create(Provider runnableProvider) { + return () -> context.runAsAdmin(() -> { + LOG.trace("create runnable from provider"); + Runnable runnable = runnableProvider.get(); + LOG.debug("execute scheduled job {}", runnable.getClass()); + runnable.run(); + }); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/schedule/QuartzScheduler.java b/scm-webapp/src/main/java/sonia/scm/schedule/QuartzScheduler.java deleted file mode 100644 index 5b46d438d8..0000000000 --- a/scm-webapp/src/main/java/sonia/scm/schedule/QuartzScheduler.java +++ /dev/null @@ -1,175 +0,0 @@ -/*** - * Copyright (c) 2015, Sebastian Sdorra - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * 3. Neither the name of SCM-Manager; nor the names of its - * contributors may be used to endorse or promote products derived from this - * software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE - * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY - * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES - * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON - * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS - * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - * https://bitbucket.org/sdorra/scm-manager - * - */ - -package sonia.scm.schedule; - -import com.google.common.base.Throwables; -import com.google.inject.Injector; -import com.google.inject.Provider; -import com.google.inject.Singleton; -import java.io.IOException; -import javax.inject.Inject; -import org.quartz.CronScheduleBuilder; -import org.quartz.JobBuilder; -import org.quartz.JobDataMap; -import org.quartz.JobDetail; -import org.quartz.SchedulerException; -import org.quartz.Trigger; -import org.quartz.TriggerBuilder; -import org.quartz.impl.StdSchedulerFactory; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import sonia.scm.Initable; -import sonia.scm.SCMContextProvider; - -/** - * {@link Scheduler} which uses the quartz scheduler. - * - * @author Sebastian Sdorra - * @since 1.47 - * - * @see Quartz Job Scheduler - */ -@Singleton -public class QuartzScheduler implements Scheduler, Initable { - - private static final Logger logger = LoggerFactory.getLogger(QuartzScheduler.class); - - private final Injector injector; - private final org.quartz.Scheduler scheduler; - - /** - * Creates a new quartz scheduler. - * - * @param injector injector - */ - @Inject - public QuartzScheduler(Injector injector) - { - this.injector = injector; - - // get default scheduler - try { - scheduler = StdSchedulerFactory.getDefaultScheduler(); - } catch (SchedulerException ex) { - throw Throwables.propagate(ex); - } - } - - /** - * Creates a new quartz scheduler. This constructor is only for testing. - * - * @param injector injector - * @param scheduler quartz scheduler - */ - QuartzScheduler(Injector injector, org.quartz.Scheduler scheduler) - { - this.injector = injector; - this.scheduler = scheduler; - } - - @Override - public void init(SCMContextProvider context) - { - try - { - if (!scheduler.isStarted()) - { - scheduler.start(); - } - } - catch (SchedulerException ex) - { - logger.error("can not start scheduler", ex); - } - } - - @Override - public void close() throws IOException - { - try - { - if (scheduler.isStarted()){ - scheduler.shutdown(); - } - } - catch (SchedulerException ex) - { - logger.error("can not stop scheduler", ex); - } - } - - @Override - public Task schedule(String expression, final Runnable runnable) - { - return schedule(expression, new Provider(){ - @Override - public Runnable get() - { - return runnable; - } - }); - } - - @Override - public Task schedule(String expression, Class runnable) - { - return schedule(expression, injector.getProvider(runnable)); - } - - private Task schedule(String expression, Provider provider){ - // create data map with injection provider for InjectionEnabledJob - JobDataMap map = new JobDataMap(); - map.put(Runnable.class.getName(), provider); - map.put(Injector.class.getName(), injector); - - // create job detail for InjectionEnabledJob with the provider for the annotated class - JobDetail detail = JobBuilder.newJob(InjectionEnabledJob.class) - .usingJobData(map) - .build(); - - // create a trigger with the cron expression from the annotation - Trigger trigger = TriggerBuilder.newTrigger() - .forJob(detail) - .withSchedule(CronScheduleBuilder.cronSchedule(expression)) - .build(); - - try { - scheduler.scheduleJob(detail, trigger); - } catch (SchedulerException ex) { - throw Throwables.propagate(ex); - } - - return new QuartzTask(scheduler, trigger.getJobKey()); - } - - -} diff --git a/scm-webapp/src/main/java/sonia/scm/schedule/QuartzTask.java b/scm-webapp/src/main/java/sonia/scm/schedule/QuartzTask.java deleted file mode 100644 index a45790a2e9..0000000000 --- a/scm-webapp/src/main/java/sonia/scm/schedule/QuartzTask.java +++ /dev/null @@ -1,68 +0,0 @@ -/*** - * Copyright (c) 2015, Sebastian Sdorra - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * 3. Neither the name of SCM-Manager; nor the names of its - * contributors may be used to endorse or promote products derived from this - * software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE - * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY - * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES - * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON - * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS - * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - * https://bitbucket.org/sdorra/scm-manager - * - */ - -package sonia.scm.schedule; - -import com.google.common.base.Throwables; -import org.quartz.JobKey; -import org.quartz.Scheduler; -import org.quartz.SchedulerException; - -/** - * Task implementation for the {@link QuartzScheduler}. - * - * @author Sebastian Sdorra - */ -public class QuartzTask implements Task { - - private final org.quartz.Scheduler scheduler; - private final JobKey jobKey; - - QuartzTask(Scheduler scheduler, JobKey jobKey) - { - this.scheduler = scheduler; - this.jobKey = jobKey; - } - - @Override - public void cancel() - { - try - { - scheduler.deleteJob(jobKey); - } - catch (SchedulerException ex) - { - throw Throwables.propagate(ex); - } - } - -} diff --git a/scm-webapp/src/test/java/sonia/scm/schedule/CronExpressionTest.java b/scm-webapp/src/test/java/sonia/scm/schedule/CronExpressionTest.java new file mode 100644 index 0000000000..6558bce781 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/schedule/CronExpressionTest.java @@ -0,0 +1,75 @@ +package sonia.scm.schedule; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.time.Clock; +import java.time.Duration; +import java.time.Instant; +import java.time.ZoneId; +import java.time.ZonedDateTime; +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class CronExpressionTest { + + @Mock + private Clock clock; + + @BeforeEach + void setUpClockMock() { + when(clock.getZone()).thenReturn(ZoneId.systemDefault()); + when(clock.instant()).thenReturn(Instant.parse("2007-12-03T10:15:00.00Z")); + } + + @Test + void shouldCalculateTheNextRunIn30Seconds() { + assertNextRun("30 * * * * ?", 30); + } + + @Test + void shouldCalculateTheNextRunIn10Seconds() { + assertNextRun("0/10 * * * * ?", 10); + } + + @Test + void shouldReturnEmptyOptional() { + CronExpression expression = new CronExpression(clock, "30 12 12 12 * ? 1985"); + + Optional optionalNextRun = expression.calculateNextRun(); + assertThat(optionalNextRun).isNotPresent(); + } + + @Test + void shouldReturnTrue() { + ZonedDateTime time = ZonedDateTime.now(clock).minusSeconds(1L); + + CronExpression expression = new CronExpression(clock, "30 * * * * ?"); + assertThat(expression.shouldRun(time)).isTrue(); + } + + @Test + void shouldReturnFalse() { + ZonedDateTime time = ZonedDateTime.now(clock).plusSeconds(1L); + + CronExpression expression = new CronExpression(clock, "30 * * * * ?"); + assertThat(expression.shouldRun(time)).isFalse(); + } + + private void assertNextRun(String expressionAsString, long expectedSecondsToNextRun) { + CronExpression expression = new CronExpression(clock, expressionAsString); + + Optional optionalNextRun = expression.calculateNextRun(); + assertThat(optionalNextRun).isPresent(); + + ZonedDateTime nextRun = optionalNextRun.get(); + long nextRunInSeconds = Duration.between(ZonedDateTime.now(clock), nextRun).getSeconds(); + assertThat(nextRunInSeconds).isEqualTo(expectedSecondsToNextRun); + } +} diff --git a/scm-webapp/src/test/java/sonia/scm/schedule/CronSchedulerTest.java b/scm-webapp/src/test/java/sonia/scm/schedule/CronSchedulerTest.java new file mode 100644 index 0000000000..622b27228d --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/schedule/CronSchedulerTest.java @@ -0,0 +1,66 @@ +package sonia.scm.schedule; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.concurrent.Future; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +class CronSchedulerTest { + + @Mock + private CronTaskFactory taskFactory; + + @Mock + private CronTask task; + + @BeforeEach + @SuppressWarnings("unchecked") + void setUpTaskFactory() { + lenient().when(taskFactory.create(anyString(), any(Runnable.class))).thenReturn(task); + lenient().when(taskFactory.create(anyString(), any(Class.class))).thenReturn(task); + } + + @Test + void shouldScheduleWithClass() { + when(task.hasNextRun()).thenReturn(true); + try (CronScheduler scheduler = new CronScheduler(taskFactory)) { + scheduler.schedule("vep", TestingRunnable.class); + verify(task).setFuture(any(Future.class)); + } + } + + @Test + void shouldScheduleWithRunnable() { + when(task.hasNextRun()).thenReturn(true); + try (CronScheduler scheduler = new CronScheduler(taskFactory)) { + scheduler.schedule("vep", new TestingRunnable()); + verify(task).setFuture(any(Future.class)); + } + } + + @Test + void shouldSkipSchedulingWithoutNextRun(){ + try (CronScheduler scheduler = new CronScheduler(taskFactory)) { + scheduler.schedule("vep", new TestingRunnable()); + verify(task, never()).setFuture(any(Future.class)); + } + } + + private static class TestingRunnable implements Runnable { + + @Override + public void run() { + + } + } + +} diff --git a/scm-webapp/src/test/java/sonia/scm/schedule/CronTaskFactoryTest.java b/scm-webapp/src/test/java/sonia/scm/schedule/CronTaskFactoryTest.java new file mode 100644 index 0000000000..fe5c299b74 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/schedule/CronTaskFactoryTest.java @@ -0,0 +1,67 @@ +package sonia.scm.schedule; + +import com.google.inject.Injector; +import com.google.inject.util.Providers; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import javax.inject.Provider; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class CronTaskFactoryTest { + + @Mock + private Injector injector; + + @Mock + private PrivilegedRunnableFactory runnableFactory; + + @InjectMocks + private CronTaskFactory taskFactory; + + @BeforeEach + @SuppressWarnings("unchecked") + void setUpMocks() { + when(runnableFactory.create(any(Provider.class))).thenAnswer(ic -> { + Provider r = ic.getArgument(0); + return r.get(); + }); + } + + @Test + void shouldCreateATaskWithNameFromRunnable() { + CronTask task = taskFactory.create("30 * * * * ?", new One()); + assertThat(task.getName()).isEqualTo(One.class.getName()); + } + + @Test + void shouldCreateATaskWithNameFromClass() { + when(injector.getProvider(One.class)).thenReturn(Providers.of(new One())); + + CronTask task = taskFactory.create("30 * * * * ?", One.class); + assertThat(task.getName()).isEqualTo(One.class.getName()); + } + + @Test + void shouldCreateATaskWithCronExpression() { + CronTask task = taskFactory.create("30 * * * * ?", new One()); + assertThat(task.getExpression().toString()).isEqualTo("30 * * * * ?"); + } + + public static class One implements Runnable { + + @Override + public void run() { + + } + } +} diff --git a/scm-webapp/src/test/java/sonia/scm/schedule/CronTaskTest.java b/scm-webapp/src/test/java/sonia/scm/schedule/CronTaskTest.java new file mode 100644 index 0000000000..7cfbba29e3 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/schedule/CronTaskTest.java @@ -0,0 +1,95 @@ +package sonia.scm.schedule; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.stubbing.Answer; + +import java.time.ZonedDateTime; +import java.util.Optional; +import java.util.concurrent.Future; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +class CronTaskTest { + + @Mock + private CronExpression expression; + + @Mock + private Runnable runnable; + + @Mock + private Future future; + + @Test + void shouldReturnTrue() { + when(expression.calculateNextRun()).thenReturn(Optional.of(ZonedDateTime.now())); + + CronTask task = task(); + + assertThat(task.hasNextRun()).isTrue(); + } + + @Test + void shouldReturnFalse() { + when(expression.calculateNextRun()).thenReturn(Optional.empty()); + + CronTask task = task(); + + assertThat(task.hasNextRun()).isFalse(); + } + + private CronTask task() { + return new CronTask("one", expression, runnable); + } + + @Test + void shouldCancelWithoutNextRun() { + ZonedDateTime time = ZonedDateTime.now(); + when(expression.calculateNextRun()).thenAnswer(new FirstTimeAnswer(Optional.of(time), Optional.empty())); + when(expression.shouldRun(time)).thenReturn(true); + + CronTask task = task(); + task.setFuture(future); + task.run(); + + verify(runnable).run(); + verify(future).cancel(false); + } + + @Test + void shouldNotRun() { + task().run(); + + verify(runnable, never()).run(); + } + + private static class FirstTimeAnswer implements Answer { + + private boolean first = true; + private final Object answer; + private final Object secondAnswer; + + FirstTimeAnswer(Object answer, Object secondAnswer) { + this.answer = answer; + this.secondAnswer = secondAnswer; + } + + @Override + public Object answer(InvocationOnMock invocation) { + if (first) { + first = false; + return answer; + } + return secondAnswer; + } + } + +} diff --git a/scm-webapp/src/test/java/sonia/scm/schedule/CronThreadFactoryTest.java b/scm-webapp/src/test/java/sonia/scm/schedule/CronThreadFactoryTest.java new file mode 100644 index 0000000000..313f007e19 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/schedule/CronThreadFactoryTest.java @@ -0,0 +1,87 @@ +package sonia.scm.schedule; + +import org.apache.shiro.subject.Subject; +import org.apache.shiro.util.ThreadContext; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Collections; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; + +@ExtendWith(MockitoExtension.class) +class CronThreadFactoryTest { + + private Runnable doNothind = () -> {}; + + @Test + void shouldCreateThreadWithName() { + try (CronThreadFactory threadFactory = new CronThreadFactory()) { + Thread thread = threadFactory.newThread(doNothind); + assertThat(thread.getName()).startsWith("CronScheduler-"); + } + } + + @Test + void shouldCreateThreadsWithDifferentNames() { + try (CronThreadFactory threadFactory = new CronThreadFactory()) { + Thread one = threadFactory.newThread(doNothind); + Thread two = threadFactory.newThread(doNothind); + assertThat(one.getName()).isNotEqualTo(two.getName()); + } + } + + @Test + void shouldCreateThreadsWithDifferentNamesFromDifferentFactories() { + String one; + try (CronThreadFactory threadFactory = new CronThreadFactory()) { + one = threadFactory.newThread(doNothind).getName(); + } + + String two; + try (CronThreadFactory threadFactory = new CronThreadFactory()) { + two = threadFactory.newThread(doNothind).getName(); + } + + assertThat(one).isNotEqualTo(two); + } + + @Nested + class ShiroTests { + + @Mock + private Subject subject; + + @BeforeEach + void setUpContext() { + ThreadContext.bind(subject); + } + + @Test + void shouldNotInheritShiroContext() throws InterruptedException { + ShiroResourceCapturingRunnable runnable = new ShiroResourceCapturingRunnable(); + try (CronThreadFactory threadFactory = new CronThreadFactory()) { + Thread thread = threadFactory.newThread(runnable); + thread.start(); + thread.join(); + } + assertThat(runnable.resources).isSameAs(Collections.emptyMap()); + } + } + + + private static class ShiroResourceCapturingRunnable implements Runnable { + + private Map resources; + + @Override + public void run() { + resources = ThreadContext.getResources(); + } + } +} diff --git a/scm-webapp/src/test/java/sonia/scm/schedule/InjectionEnabledJobTest.java b/scm-webapp/src/test/java/sonia/scm/schedule/InjectionEnabledJobTest.java deleted file mode 100644 index 88516d761c..0000000000 --- a/scm-webapp/src/test/java/sonia/scm/schedule/InjectionEnabledJobTest.java +++ /dev/null @@ -1,168 +0,0 @@ -/*** - * Copyright (c) 2015, Sebastian Sdorra - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * 3. Neither the name of SCM-Manager; nor the names of its - * contributors may be used to endorse or promote products derived from this - * software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE - * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY - * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES - * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON - * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS - * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - * https://bitbucket.org/sdorra/scm-manager - * - */ - -package sonia.scm.schedule; - -import com.google.inject.Injector; -import com.google.inject.Provider; -import org.junit.Test; -import static org.mockito.Mockito.*; -import org.junit.Rule; -import org.junit.rules.ExpectedException; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.junit.MockitoJUnitRunner; -import org.quartz.JobDataMap; -import org.quartz.JobDetail; -import org.quartz.JobExecutionContext; -import org.quartz.JobExecutionException; -import sonia.scm.web.security.AdministrationContext; -import sonia.scm.web.security.PrivilegedAction; - -/** - * Unit tests for {@link InjectionEnabledJob}. - * - * @author Sebastian Sdorra - */ -@RunWith(MockitoJUnitRunner.class) -public class InjectionEnabledJobTest { - - @Mock - private Injector injector; - - @Mock - private JobDataMap dataMap; - - @Mock - private JobDetail detail; - - @Mock - private JobExecutionContext jec; - - @Mock - private Provider runnable; - - @Mock - private AdministrationContext context; - - @Rule - public ExpectedException expected = ExpectedException.none(); - - /** - * Tests {@link InjectionEnabledJob#execute(org.quartz.JobExecutionContext)} without context. - * - * @throws JobExecutionException - */ - @Test - public void testExecuteWithoutContext() throws JobExecutionException - { - expected.expect(NullPointerException.class); - expected.expectMessage("execution context"); - new InjectionEnabledJob().execute(null); - } - - /** - * Tests {@link InjectionEnabledJob#execute(org.quartz.JobExecutionContext)} without job detail. - * - * @throws JobExecutionException - */ - @Test - public void testExecuteWithoutJobDetail() throws JobExecutionException - { - expected.expect(NullPointerException.class); - expected.expectMessage("detail"); - new InjectionEnabledJob().execute(jec); - } - - /** - * Tests {@link InjectionEnabledJob#execute(org.quartz.JobExecutionContext)} without data map. - * - * @throws JobExecutionException - */ - @Test - public void testExecuteWithoutDataMap() throws JobExecutionException - { - when(jec.getJobDetail()).thenReturn(detail); - expected.expect(NullPointerException.class); - expected.expectMessage("data map"); - new InjectionEnabledJob().execute(jec); - } - - /** - * Tests {@link InjectionEnabledJob#execute(org.quartz.JobExecutionContext)} without injector. - * - * @throws JobExecutionException - */ - @Test - public void testExecuteWithoutInjector() throws JobExecutionException - { - when(jec.getJobDetail()).thenReturn(detail); - when(detail.getJobDataMap()).thenReturn(dataMap); - expected.expect(NullPointerException.class); - expected.expectMessage("injector"); - new InjectionEnabledJob().execute(jec); - } - - /** - * Tests {@link InjectionEnabledJob#execute(org.quartz.JobExecutionContext)} without runnable. - * - * @throws JobExecutionException - */ - @Test - public void testExecuteWithoutRunnable() throws JobExecutionException - { - when(jec.getJobDetail()).thenReturn(detail); - when(detail.getJobDataMap()).thenReturn(dataMap); - when(dataMap.get(Injector.class.getName())).thenReturn(injector); - expected.expect(JobExecutionException.class); - expected.expectMessage("runnable"); - new InjectionEnabledJob().execute(jec); - } - - /** - * Tests {@link InjectionEnabledJob#execute(org.quartz.JobExecutionContext)}. - * - * @throws JobExecutionException - */ - @Test - public void testExecute() throws JobExecutionException - { - when(jec.getJobDetail()).thenReturn(detail); - when(detail.getJobDataMap()).thenReturn(dataMap); - when(dataMap.get(Injector.class.getName())).thenReturn(injector); - when(dataMap.get(Runnable.class.getName())).thenReturn(runnable); - when(injector.getInstance(AdministrationContext.class)).thenReturn(context); - new InjectionEnabledJob().execute(jec); - verify(context).runAsAdmin(Mockito.any(PrivilegedAction.class)); - } - -} diff --git a/scm-webapp/src/test/java/sonia/scm/schedule/PrivilegedRunnableFactoryTest.java b/scm-webapp/src/test/java/sonia/scm/schedule/PrivilegedRunnableFactoryTest.java new file mode 100644 index 0000000000..66c87c5a37 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/schedule/PrivilegedRunnableFactoryTest.java @@ -0,0 +1,53 @@ +package sonia.scm.schedule; + +import com.google.inject.util.Providers; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import sonia.scm.web.security.AdministrationContext; +import sonia.scm.web.security.PrivilegedAction; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; + +@ExtendWith(MockitoExtension.class) +class PrivilegedRunnableFactoryTest { + + @Mock + private AdministrationContext administrationContext; + + @InjectMocks + private PrivilegedRunnableFactory runnableFactory; + + @Test + void shouldRunAsPrivilegedAction() { + doAnswer((ic) -> { + PrivilegedAction action = ic.getArgument(0); + action.run(); + return null; + }).when(administrationContext).runAsAdmin(any(PrivilegedAction.class)); + + RemindingRunnable runnable = new RemindingRunnable(); + + Runnable action = runnableFactory.create(Providers.of(runnable)); + assertThat(action).isNotExactlyInstanceOf(RemindingRunnable.class); + + assertThat(runnable.run).isFalse(); + action.run(); + assertThat(runnable.run).isTrue(); + } + + private static class RemindingRunnable implements Runnable { + + private boolean run = false; + + @Override + public void run() { + run = true; + } + } + +} diff --git a/scm-webapp/src/test/java/sonia/scm/schedule/QuartzSchedulerTest.java b/scm-webapp/src/test/java/sonia/scm/schedule/QuartzSchedulerTest.java deleted file mode 100644 index cd272ee7ba..0000000000 --- a/scm-webapp/src/test/java/sonia/scm/schedule/QuartzSchedulerTest.java +++ /dev/null @@ -1,222 +0,0 @@ -/*** - * Copyright (c) 2015, Sebastian Sdorra - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * 3. Neither the name of SCM-Manager; nor the names of its - * contributors may be used to endorse or promote products derived from this - * software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE - * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY - * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES - * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON - * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS - * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - * https://bitbucket.org/sdorra/scm-manager - * - */ - -package sonia.scm.schedule; - -import com.google.inject.Injector; -import com.google.inject.Provider; -import java.io.IOException; -import org.junit.Test; -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; -import static org.hamcrest.Matchers.*; -import org.junit.Before; -import org.junit.runner.RunWith; -import org.mockito.ArgumentCaptor; -import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; -import org.quartz.CronTrigger; -import org.quartz.JobDetail; -import org.quartz.SchedulerException; -import org.quartz.Trigger; - -/** - * Unit tests for {@link QuartzScheduler}. - * - * @author Sebastian Sdorra - */ -@RunWith(MockitoJUnitRunner.class) -public class QuartzSchedulerTest { - - @Mock - private Injector injector; - - @Mock - private org.quartz.Scheduler quartzScheduler; - - private QuartzScheduler scheduler; - - @Before - public void setUp() - { - scheduler = new QuartzScheduler(injector, quartzScheduler); - } - - /** - * Tests {@link QuartzScheduler#schedule(java.lang.String, java.lang.Runnable)}. - * - * @throws SchedulerException - */ - @Test - @SuppressWarnings("unchecked") - public void testSchedule() throws SchedulerException - { - DummyRunnable dr = new DummyRunnable(); - Task task = scheduler.schedule("42 2 * * * ?", dr); - assertNotNull(task); - - ArgumentCaptor detailCaptor = ArgumentCaptor.forClass(JobDetail.class); - ArgumentCaptor triggerCaptor = ArgumentCaptor.forClass(Trigger.class); - verify(quartzScheduler).scheduleJob(detailCaptor.capture(), triggerCaptor.capture()); - - Trigger trigger = triggerCaptor.getValue(); - assertThat(trigger, is(instanceOf(CronTrigger.class))); - CronTrigger cron = (CronTrigger) trigger; - assertEquals("42 2 * * * ?", cron.getCronExpression()); - - JobDetail detail = detailCaptor.getValue(); - assertEquals(InjectionEnabledJob.class, detail.getJobClass()); - Provider runnable = (Provider) detail.getJobDataMap().get(Runnable.class.getName()); - assertNotNull(runnable); - assertSame(dr, runnable.get()); - assertEquals(injector, detail.getJobDataMap().get(Injector.class.getName())); - } - - /** - * Tests {@link QuartzScheduler#schedule(java.lang.String, java.lang.Class)}. - * - * @throws SchedulerException - */ - @Test - public void testScheduleWithClass() throws SchedulerException - { - scheduler.schedule("42 * * * * ?", DummyRunnable.class); - - verify(injector).getProvider(DummyRunnable.class); - - ArgumentCaptor detailCaptor = ArgumentCaptor.forClass(JobDetail.class); - ArgumentCaptor triggerCaptor = ArgumentCaptor.forClass(Trigger.class); - verify(quartzScheduler).scheduleJob(detailCaptor.capture(), triggerCaptor.capture()); - - Trigger trigger = triggerCaptor.getValue(); - assertThat(trigger, is(instanceOf(CronTrigger.class))); - CronTrigger cron = (CronTrigger) trigger; - assertEquals("42 * * * * ?", cron.getCronExpression()); - - JobDetail detail = detailCaptor.getValue(); - assertEquals(InjectionEnabledJob.class, detail.getJobClass()); - assertEquals(injector, detail.getJobDataMap().get(Injector.class.getName())); - } - - /** - * Tests {@link QuartzScheduler#init(sonia.scm.SCMContextProvider)}. - * - * @throws SchedulerException - */ - @Test - public void testInit() throws SchedulerException - { - when(quartzScheduler.isStarted()).thenReturn(Boolean.FALSE); - scheduler.init(null); - verify(quartzScheduler).start(); - } - - /** - * Tests {@link QuartzScheduler#init(sonia.scm.SCMContextProvider)} when the underlying scheduler is already started. - * - * @throws SchedulerException - */ - @Test - public void testInitAlreadyRunning() throws SchedulerException - { - when(quartzScheduler.isStarted()).thenReturn(Boolean.TRUE); - scheduler.init(null); - verify(quartzScheduler, never()).start(); - } - - /** - * Tests {@link QuartzScheduler#init(sonia.scm.SCMContextProvider)} when the underlying scheduler throws an exception. - * - * @throws SchedulerException - */ - @Test - @SuppressWarnings("unchecked") - public void testInitException() throws SchedulerException - { - when(quartzScheduler.isStarted()).thenThrow(SchedulerException.class); - scheduler.init(null); - verify(quartzScheduler, never()).start(); - } - - /** - * Tests {@link QuartzScheduler#close()}. - * - * @throws IOException - * @throws SchedulerException - */ - @Test - public void testClose() throws IOException, SchedulerException - { - when(quartzScheduler.isStarted()).thenReturn(Boolean.TRUE); - scheduler.close(); - verify(quartzScheduler).shutdown(); - } - - /** - * Tests {@link QuartzScheduler#close()} when the underlying scheduler is not running. - * - * @throws IOException - * @throws SchedulerException - */ - @Test - public void testCloseNotRunning() throws IOException, SchedulerException - { - when(quartzScheduler.isStarted()).thenReturn(Boolean.FALSE); - scheduler.close(); - verify(quartzScheduler, never()).shutdown(); - } - - /** - * Tests {@link QuartzScheduler#close()} when the underlying scheduler throws an exception. - * - * @throws IOException - * @throws SchedulerException - */ - @Test - @SuppressWarnings("unchecked") - public void testCloseException() throws IOException, SchedulerException - { - when(quartzScheduler.isStarted()).thenThrow(SchedulerException.class); - scheduler.close(); - verify(quartzScheduler, never()).shutdown(); - } - - - public static class DummyRunnable implements Runnable { - - @Override - public void run() - { - throw new UnsupportedOperationException("Not supported yet."); //To change body of generated methods, choose Tools | Templates. - } - - } -} diff --git a/scm-webapp/src/test/java/sonia/scm/schedule/QuartzTaskTest.java b/scm-webapp/src/test/java/sonia/scm/schedule/QuartzTaskTest.java deleted file mode 100644 index baf4c659cc..0000000000 --- a/scm-webapp/src/test/java/sonia/scm/schedule/QuartzTaskTest.java +++ /dev/null @@ -1,89 +0,0 @@ -/*** - * Copyright (c) 2015, Sebastian Sdorra - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * 3. Neither the name of SCM-Manager; nor the names of its - * contributors may be used to endorse or promote products derived from this - * software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE - * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY - * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES - * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON - * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS - * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - * https://bitbucket.org/sdorra/scm-manager - * - */ - -package sonia.scm.schedule; - -import org.junit.Test; - -import static org.mockito.Mockito.*; - -import org.junit.Before; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; -import org.quartz.JobKey; -import org.quartz.SchedulerException; - -/** - * Unit tests for {@link QuartzTask}. - * - * @author Sebastian Sdorra - */ -@RunWith(MockitoJUnitRunner.class) -public class QuartzTaskTest { - - @Mock - private org.quartz.Scheduler scheduler; - - private final JobKey jobKey = new JobKey("sample"); - - private QuartzTask task; - - @Before - public void setUp(){ - task = new QuartzTask(scheduler, jobKey); - } - - /** - * Tests {@link QuartzTask#cancel()}. - * - * @throws SchedulerException - */ - @Test - public void testCancel() throws SchedulerException - { - task.cancel(); - verify(scheduler).deleteJob(jobKey); - } - - /** - * Tests {@link QuartzTask#cancel()} when the scheduler throws an exception. - * @throws org.quartz.SchedulerException - */ - @SuppressWarnings("unchecked") - @Test(expected = RuntimeException.class) - public void testCancelWithException() throws SchedulerException - { - when(scheduler.deleteJob(jobKey)).thenThrow(SchedulerException.class); - task.cancel(); - } - -} From ece1a2b34a00f8ebe3d771ffff1941e8e988f54a Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 19 Jun 2019 11:40:36 +0200 Subject: [PATCH 06/16] update jgit to v5.4.0.201906121030-r-scm1 --- pom.xml | 2 +- .../java/sonia/scm/web/lfs/servlet/ScmLfsProtocolServlet.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index e2cd118241..43312cba64 100644 --- a/pom.xml +++ b/pom.xml @@ -837,7 +837,7 @@ 1.4.0 - v4.5.3.201708160445-r-scm1 + v5.4.0.201906121030-r-scm1 1.9.0-scm3 diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/ScmLfsProtocolServlet.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/ScmLfsProtocolServlet.java index 332cf12e09..5c63cad571 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/ScmLfsProtocolServlet.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/ScmLfsProtocolServlet.java @@ -20,7 +20,7 @@ public class ScmLfsProtocolServlet extends LfsProtocolServlet { @Override - protected LargeFileRepository getLargeFileRepository(LfsRequest request, String path) throws LfsException { + protected LargeFileRepository getLargeFileRepository(LfsRequest request, String path, String auth) throws LfsException { return repository; } } From b7af4fa902726acc7a4cf774222b741bae95c931 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 19 Jun 2019 11:41:36 +0200 Subject: [PATCH 07/16] shutdown jgit workqueue, when context is destroyed --- .../GitWorkQueueShutdownListener.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkQueueShutdownListener.java diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkQueueShutdownListener.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkQueueShutdownListener.java new file mode 100644 index 0000000000..e3dcd7e564 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkQueueShutdownListener.java @@ -0,0 +1,26 @@ +package sonia.scm.repository; + +import org.eclipse.jgit.lib.internal.WorkQueue; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import sonia.scm.plugin.Extension; + +import javax.servlet.ServletContextEvent; +import javax.servlet.ServletContextListener; + +@Extension +public class GitWorkQueueShutdownListener implements ServletContextListener { + + private static final Logger LOG = LoggerFactory.getLogger(GitWorkQueueShutdownListener.class); + + @Override + public void contextInitialized(ServletContextEvent sce) { + + } + + @Override + public void contextDestroyed(ServletContextEvent sce) { + LOG.warn("shutdown jGit WorkQueue executor"); + WorkQueue.getExecutor().shutdown(); + } +} From 6cee35a9f1fe8652c7274665dd9cf76369336e01 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 19 Jun 2019 11:46:49 +0200 Subject: [PATCH 08/16] update legman to avoid already shutdown exception, during restart --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 43312cba64..ba06282720 100644 --- a/pom.xml +++ b/pom.xml @@ -826,7 +826,7 @@ 2.3.0 - 1.5.0 + 1.5.1 9.4.14.v20181114 From 91fd259f07947e57674f27f258c418873e4b76ee Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 19 Jun 2019 11:52:20 +0200 Subject: [PATCH 09/16] use ClassLoaderLeakPreventor to reduce ClassLoaderLeaks of plugins --- scm-webapp/pom.xml | 8 ++ .../java/sonia/scm/ScmContextListener.java | 15 --- .../sonia/scm/boot/BootstrapClassLoader.java | 11 ++ .../scm/boot/BootstrapContextListener.java | 14 +- .../sonia/scm/boot/ClassLoaderLifeCycle.java | 124 ++++++++++++++++++ .../java/sonia/scm/boot/LoggingAdapter.java | 43 ++++++ .../plugin/ChildFirstPluginClassLoader.java | 19 ++- .../scm/plugin/DefaultPluginClassLoader.java | 19 ++- .../sonia/scm/plugin/PluginProcessor.java | 20 +-- .../sonia/scm/plugin/PluginsInternal.java | 9 +- .../scm/boot/ClassLoaderLifeCycleTest.java | 112 ++++++++++++++++ .../sonia/scm/plugin/PluginProcessorTest.java | 3 +- 12 files changed, 341 insertions(+), 56 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/boot/BootstrapClassLoader.java create mode 100644 scm-webapp/src/main/java/sonia/scm/boot/ClassLoaderLifeCycle.java create mode 100644 scm-webapp/src/main/java/sonia/scm/boot/LoggingAdapter.java create mode 100644 scm-webapp/src/test/java/sonia/scm/boot/ClassLoaderLifeCycleTest.java diff --git a/scm-webapp/pom.xml b/scm-webapp/pom.xml index 490dac3123..cc2d9b99da 100644 --- a/scm-webapp/pom.xml +++ b/scm-webapp/pom.xml @@ -276,6 +276,14 @@ 1.20 + + + + se.jiderhamn.classloader-leak-prevention + classloader-leak-prevention-core + 2.7.0 + + diff --git a/scm-webapp/src/main/java/sonia/scm/ScmContextListener.java b/scm-webapp/src/main/java/sonia/scm/ScmContextListener.java index cc58d34ef3..cb57e88933 100644 --- a/scm-webapp/src/main/java/sonia/scm/ScmContextListener.java +++ b/scm-webapp/src/main/java/sonia/scm/ScmContextListener.java @@ -58,9 +58,6 @@ import sonia.scm.util.IOUtil; import javax.inject.Inject; import javax.servlet.ServletContext; import javax.servlet.ServletContextEvent; -import java.io.Closeable; -import java.io.IOException; -import java.util.Collections; import java.util.List; import java.util.Set; @@ -174,18 +171,6 @@ public class ScmContextListener extends GuiceResteasyBootstrapServletContextList } super.contextDestroyed(servletContextEvent); - - for (PluginWrapper plugin : getPlugins()) { - ClassLoader pcl = plugin.getClassLoader(); - - if (pcl instanceof Closeable) { - try { - ((Closeable) pcl).close(); - } catch (IOException ex) { - LOG.warn("could not close plugin classloader", ex); - } - } - } } private void closeCloseables() { diff --git a/scm-webapp/src/main/java/sonia/scm/boot/BootstrapClassLoader.java b/scm-webapp/src/main/java/sonia/scm/boot/BootstrapClassLoader.java new file mode 100644 index 0000000000..cc7b807137 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/boot/BootstrapClassLoader.java @@ -0,0 +1,11 @@ +package sonia.scm.boot; + +/** + * This ClassLoader is mainly a wrapper around the web application class loader and its goal is to make it easier to + * find it in a heap dump. + */ +class BootstrapClassLoader extends ClassLoader { + BootstrapClassLoader(ClassLoader webappClassLoader) { + super(webappClassLoader); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextListener.java b/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextListener.java index 378016a8fd..572ff99d49 100644 --- a/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextListener.java +++ b/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextListener.java @@ -55,7 +55,6 @@ import sonia.scm.plugin.PluginsInternal; import sonia.scm.plugin.SmpArchive; import sonia.scm.update.MigrationWizardContextListener; import sonia.scm.update.UpdateEngine; -import sonia.scm.util.ClassLoaders; import sonia.scm.util.IOUtil; import javax.servlet.ServletContext; @@ -100,6 +99,8 @@ public class BootstrapContextListener implements ServletContextListener { //~--- methods -------------------------------------------------------------- + private final ClassLoaderLifeCycle classLoaderLifeCycle = ClassLoaderLifeCycle.create(); + /** * Method description * @@ -109,6 +110,7 @@ public class BootstrapContextListener implements ServletContextListener { @Override public void contextDestroyed(ServletContextEvent sce) { contextListener.contextDestroyed(sce); + classLoaderLifeCycle.shutdown(); context = null; contextListener = null; @@ -122,6 +124,8 @@ public class BootstrapContextListener implements ServletContextListener { */ @Override public void contextInitialized(ServletContextEvent sce) { + classLoaderLifeCycle.init(); + context = sce.getServletContext(); createContextListener(); @@ -142,7 +146,6 @@ public class BootstrapContextListener implements ServletContextListener { } private void createMigrationOrNormalContextListener() { - ClassLoader cl; Set plugins; PluginLoader pluginLoader; @@ -157,11 +160,10 @@ public class BootstrapContextListener implements ServletContextListener { logger.info("core plugin extraction is disabled"); } - cl = ClassLoaders.getContextClassLoader(BootstrapContextListener.class); - plugins = PluginsInternal.collectPlugins(cl, pluginDirectory.toPath()); + plugins = PluginsInternal.collectPlugins(classLoaderLifeCycle, pluginDirectory.toPath()); - pluginLoader = new DefaultPluginLoader(context, cl, plugins); + pluginLoader = new DefaultPluginLoader(context, classLoaderLifeCycle.getBootstrapClassLoader(), plugins); } catch (IOException ex) { throw new PluginLoadException("could not load plugins", ex); @@ -169,7 +171,7 @@ public class BootstrapContextListener implements ServletContextListener { Injector bootstrapInjector = createBootstrapInjector(pluginLoader); - startEitherMigrationOrNormalServlet(cl, plugins, pluginLoader, bootstrapInjector); + startEitherMigrationOrNormalServlet(classLoaderLifeCycle.getBootstrapClassLoader(), plugins, pluginLoader, bootstrapInjector); } private void startEitherMigrationOrNormalServlet(ClassLoader cl, Set plugins, PluginLoader pluginLoader, Injector bootstrapInjector) { diff --git a/scm-webapp/src/main/java/sonia/scm/boot/ClassLoaderLifeCycle.java b/scm-webapp/src/main/java/sonia/scm/boot/ClassLoaderLifeCycle.java new file mode 100644 index 0000000000..648197b7e4 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/boot/ClassLoaderLifeCycle.java @@ -0,0 +1,124 @@ +package sonia.scm.boot; + +import com.google.common.annotations.VisibleForTesting; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import se.jiderhamn.classloader.leak.prevention.ClassLoaderLeakPreventor; +import se.jiderhamn.classloader.leak.prevention.ClassLoaderLeakPreventorFactory; +import sonia.scm.plugin.ChildFirstPluginClassLoader; +import sonia.scm.plugin.DefaultPluginClassLoader; + +import java.io.Closeable; +import java.io.IOException; +import java.net.URL; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.function.UnaryOperator; + +import static com.google.common.base.Preconditions.checkState; + +/** + * Creates and shutdown SCM-Manager ClassLoaders. + */ +public final class ClassLoaderLifeCycle { + + private static final Logger LOG = LoggerFactory.getLogger(ClassLoaderLifeCycle.class); + + private final Deque classLoaders = new ArrayDeque<>(); + + private final ClassLoaderLeakPreventorFactory classLoaderLeakPreventorFactory; + private final ClassLoader webappClassLoader; + + private ClassLoader bootstrapClassLoader; + private UnaryOperator classLoaderAppendListener = c -> c; + + @VisibleForTesting + public static ClassLoaderLifeCycle create() { + ClassLoaderLeakPreventorFactory classLoaderLeakPreventorFactory = new ClassLoaderLeakPreventorFactory(); + classLoaderLeakPreventorFactory.setLogger(new LoggingAdapter()); + return new ClassLoaderLifeCycle(Thread.currentThread().getContextClassLoader(), classLoaderLeakPreventorFactory); + } + + ClassLoaderLifeCycle(ClassLoader webappClassLoader, ClassLoaderLeakPreventorFactory classLoaderLeakPreventorFactory) { + this.classLoaderLeakPreventorFactory = classLoaderLeakPreventorFactory; + this.webappClassLoader = initAndAppend(webappClassLoader); + } + + void init() { + bootstrapClassLoader = initAndAppend(new BootstrapClassLoader(webappClassLoader)); + } + + @VisibleForTesting + void setClassLoaderAppendListener(UnaryOperator classLoaderAppendListener) { + this.classLoaderAppendListener = classLoaderAppendListener; + } + + public ClassLoader getBootstrapClassLoader() { + checkState(bootstrapClassLoader != null, "%s was not initialized", ClassLoaderLifeCycle.class.getName()); + return bootstrapClassLoader; + } + + public ClassLoader createPluginClassLoader(URL[] urls, ClassLoader parent, String plugin) { + LOG.debug("create new PluginClassLoader for {}", plugin); + DefaultPluginClassLoader pluginClassLoader = new DefaultPluginClassLoader(urls, parent, plugin); + return initAndAppend(pluginClassLoader); + } + + public ClassLoader createChildFirstPluginClassLoader(URL[] urls, ClassLoader parent, String plugin) { + LOG.debug("create new ChildFirstPluginClassLoader for {}", plugin); + ChildFirstPluginClassLoader pluginClassLoader = new ChildFirstPluginClassLoader(urls, parent, plugin); + return initAndAppend(pluginClassLoader); + } + + void shutdown() { + LOG.info("shutdown classloader infrastructure"); + ClassLoaderAndPreventor clap = classLoaders.poll(); + while (clap != null) { + clap.shutdown(); + clap = classLoaders.poll(); + } + bootstrapClassLoader = null; + } + + private ClassLoader initAndAppend(ClassLoader originalClassLoader) { + LOG.debug("init classloader {}", originalClassLoader); + ClassLoader classLoader = classLoaderAppendListener.apply(originalClassLoader); + + ClassLoaderLeakPreventor preventor = classLoaderLeakPreventorFactory.newLeakPreventor(classLoader); + preventor.runPreClassLoaderInitiators(); + classLoaders.push(new ClassLoaderAndPreventor(classLoader, preventor)); + + return classLoader; + } + + private class ClassLoaderAndPreventor { + + private final ClassLoader classLoader; + private final ClassLoaderLeakPreventor preventor; + + private ClassLoaderAndPreventor(ClassLoader classLoader, ClassLoaderLeakPreventor preventor) { + this.classLoader = classLoader; + this.preventor = preventor; + } + + void shutdown() { + LOG.debug("shutdown classloader {}", classLoader); + preventor.runCleanUps(); + + if (classLoader != webappClassLoader) { + close(); + } + } + + private void close() { + if (classLoader instanceof Closeable) { + LOG.trace("close classloader {}", classLoader); + try { + ((Closeable) classLoader).close(); + } catch (IOException e) { + LOG.warn("failed to close classloader", e); + } + } + } + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/boot/LoggingAdapter.java b/scm-webapp/src/main/java/sonia/scm/boot/LoggingAdapter.java new file mode 100644 index 0000000000..5c760f3081 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/boot/LoggingAdapter.java @@ -0,0 +1,43 @@ +package sonia.scm.boot; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import se.jiderhamn.classloader.leak.prevention.ClassLoaderLeakPreventor; + +/** + * Logging adapter for {@link ClassLoaderLeakPreventor}. + */ +public class LoggingAdapter implements se.jiderhamn.classloader.leak.prevention.Logger { + + private static final Logger LOG = LoggerFactory.getLogger(ClassLoaderLeakPreventor.class); + + @Override + public void debug(String msg) { + LOG.debug(msg); + } + + @Override + public void info(String msg) { + LOG.info(msg); + } + + @Override + public void warn(String msg) { + LOG.warn(msg); + } + + @Override + public void warn(Throwable t) { + LOG.warn(t.getMessage(), t); + } + + @Override + public void error(String msg) { + LOG.error(msg); + } + + @Override + public void error(Throwable t) { + LOG.error(t.getMessage(), t); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/ChildFirstPluginClassLoader.java b/scm-webapp/src/main/java/sonia/scm/plugin/ChildFirstPluginClassLoader.java index 5e57825401..c356e95f16 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/ChildFirstPluginClassLoader.java +++ b/scm-webapp/src/main/java/sonia/scm/plugin/ChildFirstPluginClassLoader.java @@ -48,16 +48,7 @@ public class ChildFirstPluginClassLoader extends ChildFirstURLClassLoader implements PluginClassLoader { - /** - * Constructs ... - * - * - * @param urls - */ - public ChildFirstPluginClassLoader(URL[] urls) - { - super(urls); - } + private final String plugin; /** * Constructs ... @@ -66,8 +57,14 @@ public class ChildFirstPluginClassLoader extends ChildFirstURLClassLoader * @param urls * @param parent */ - public ChildFirstPluginClassLoader(URL[] urls, ClassLoader parent) + public ChildFirstPluginClassLoader(URL[] urls, ClassLoader parent, String plugin) { super(urls, parent); + this.plugin = plugin; + } + + @Override + public String toString() { + return ChildFirstPluginClassLoader.class.getName() + " for plugin " + plugin; } } diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/DefaultPluginClassLoader.java b/scm-webapp/src/main/java/sonia/scm/plugin/DefaultPluginClassLoader.java index 532fb1dbff..5105fa385b 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/DefaultPluginClassLoader.java +++ b/scm-webapp/src/main/java/sonia/scm/plugin/DefaultPluginClassLoader.java @@ -46,16 +46,7 @@ public class DefaultPluginClassLoader extends URLClassLoader implements PluginClassLoader { - /** - * Constructs ... - * - * - * @param urls - */ - public DefaultPluginClassLoader(URL[] urls) - { - super(urls); - } + private final String plugin; /** * Constructs ... @@ -64,8 +55,14 @@ public class DefaultPluginClassLoader extends URLClassLoader * @param urls * @param parent */ - public DefaultPluginClassLoader(URL[] urls, ClassLoader parent) + public DefaultPluginClassLoader(URL[] urls, ClassLoader parent, String plugin) { super(urls, parent); + this.plugin = plugin; + } + + @Override + public String toString() { + return DefaultPluginClassLoader.class.getName() + " for plugin " + plugin; } } diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java b/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java index 11308789f4..f75329cf65 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java +++ b/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java @@ -41,6 +41,7 @@ import com.google.common.collect.Sets; import com.google.common.hash.Hashing; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import sonia.scm.boot.ClassLoaderLifeCycle; import sonia.scm.plugin.ExplodedSmp.PathTransformer; import javax.xml.bind.JAXBContext; @@ -105,14 +106,18 @@ public final class PluginProcessor //~--- constructors --------------------------------------------------------- + private ClassLoaderLifeCycle classLoaderLifeCycle; + /** * Constructs ... * * + * @param classLoaderLifeCycle * @param pluginDirectory */ - public PluginProcessor(Path pluginDirectory) + public PluginProcessor(ClassLoaderLifeCycle classLoaderLifeCycle, Path pluginDirectory) { + this.classLoaderLifeCycle = classLoaderLifeCycle; this.pluginDirectory = pluginDirectory; this.installedDirectory = findInstalledDirectory(); @@ -372,18 +377,17 @@ public final class PluginProcessor URL[] urlArray = urls.toArray(new URL[urls.size()]); Plugin plugin = smp.getPlugin(); + String id = plugin.getInformation().getId(false); + if (smp.getPlugin().isChildFirstClassLoader()) { - logger.debug("create child fist classloader for plugin {}", - plugin.getInformation().getId()); - classLoader = new ChildFirstPluginClassLoader(urlArray, - parentClassLoader); + logger.debug("create child fist classloader for plugin {}", id); + classLoader = classLoaderLifeCycle.createChildFirstPluginClassLoader(urlArray, parentClassLoader, id); } else { - logger.debug("create parent fist classloader for plugin {}", - plugin.getInformation().getId()); - classLoader = new DefaultPluginClassLoader(urlArray, parentClassLoader); + logger.debug("create parent fist classloader for plugin {}", id); + classLoader = classLoaderLifeCycle.createPluginClassLoader(urlArray, parentClassLoader, id); } return classLoader; diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/PluginsInternal.java b/scm-webapp/src/main/java/sonia/scm/plugin/PluginsInternal.java index d4706e4d5e..07d0fe4ee9 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/PluginsInternal.java +++ b/scm-webapp/src/main/java/sonia/scm/plugin/PluginsInternal.java @@ -41,6 +41,7 @@ import com.google.common.io.Files; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import sonia.scm.boot.ClassLoaderLifeCycle; import sonia.scm.util.IOUtil; //~--- JDK imports ------------------------------------------------------------ @@ -86,13 +87,13 @@ public final class PluginsInternal * * @throws IOException */ - public static Set collectPlugins(ClassLoader classLoader, - Path directory) + public static Set collectPlugins(ClassLoaderLifeCycle classLoaderLifeCycle, + Path directory) throws IOException { - PluginProcessor processor = new PluginProcessor(directory); + PluginProcessor processor = new PluginProcessor(classLoaderLifeCycle, directory); - return processor.collectPlugins(classLoader); + return processor.collectPlugins(classLoaderLifeCycle.getBootstrapClassLoader()); } /** diff --git a/scm-webapp/src/test/java/sonia/scm/boot/ClassLoaderLifeCycleTest.java b/scm-webapp/src/test/java/sonia/scm/boot/ClassLoaderLifeCycleTest.java new file mode 100644 index 0000000000..df42b2eac5 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/boot/ClassLoaderLifeCycleTest.java @@ -0,0 +1,112 @@ +package sonia.scm.boot; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import se.jiderhamn.classloader.leak.prevention.ClassLoaderLeakPreventor; +import se.jiderhamn.classloader.leak.prevention.ClassLoaderLeakPreventorFactory; + +import java.io.Closeable; +import java.io.IOException; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +class ClassLoaderLifeCycleTest { + + @Mock + private ClassLoaderLeakPreventorFactory classLoaderLeakPreventorFactory; + + @Mock + private ClassLoaderLeakPreventor classLoaderLeakPreventor; + + @Test + void shouldThrowIllegalStateExceptionWithoutInit() { + ClassLoaderLifeCycle lifeCycle = ClassLoaderLifeCycle.create(); + assertThrows(IllegalStateException.class, lifeCycle::getBootstrapClassLoader); + } + + @Test + void shouldThrowIllegalStateExceptionAfterShutdown() { + ClassLoaderLifeCycle lifeCycle = createMockedLifeCycle(); + lifeCycle.init(); + + lifeCycle.shutdown(); + assertThrows(IllegalStateException.class, lifeCycle::getBootstrapClassLoader); + } + + @Test + void shouldCreateBootstrapClassLoaderOnInit() { + ClassLoaderLifeCycle lifeCycle = ClassLoaderLifeCycle.create(); + lifeCycle.init(); + + assertThat(lifeCycle.getBootstrapClassLoader()).isNotNull(); + } + + @Test + void shouldCallTheLeakPreventor() { + ClassLoaderLifeCycle lifeCycle = createMockedLifeCycle(); + + lifeCycle.init(); + verify(classLoaderLeakPreventor, times(2)).runPreClassLoaderInitiators(); + + lifeCycle.createChildFirstPluginClassLoader(new URL[0], null, "a"); + lifeCycle.createPluginClassLoader(new URL[0], null, "b"); + verify(classLoaderLeakPreventor, times(4)).runPreClassLoaderInitiators(); + + lifeCycle.shutdown(); + verify(classLoaderLeakPreventor, times(4)).runCleanUps(); + } + + @Test + void shouldCloseCloseableClassLoaders() throws IOException { + // we use URLClassLoader, because we must be sure that the classloader is closable + URLClassLoader webappClassLoader = spy(new URLClassLoader(new URL[0], Thread.currentThread().getContextClassLoader())); + + ClassLoaderLifeCycle lifeCycle = createMockedLifeCycle(webappClassLoader); + lifeCycle.setClassLoaderAppendListener(c -> spy(c)); + lifeCycle.init(); + + ClassLoader pluginA = lifeCycle.createChildFirstPluginClassLoader(new URL[0], null, "a"); + ClassLoader pluginB = lifeCycle.createPluginClassLoader(new URL[0], null, "b"); + + lifeCycle.shutdown(); + + closed(pluginB); + closed(pluginA); + + neverClosed(webappClassLoader); + } + + private void neverClosed(Object object) throws IOException { + Closeable closeable = closeable(object); + verify(closeable, never()).close(); + } + + private void closed(Object object) throws IOException { + Closeable closeable = closeable(object); + verify(closeable).close(); + } + + private Closeable closeable(Object object) { + assertThat(object).isInstanceOf(Closeable.class); + return (Closeable) object; + } + + private ClassLoaderLifeCycle createMockedLifeCycle() { + return createMockedLifeCycle(Thread.currentThread().getContextClassLoader()); + } + + private ClassLoaderLifeCycle createMockedLifeCycle(ClassLoader classLoader) { + when(classLoaderLeakPreventorFactory.newLeakPreventor(any(ClassLoader.class))).thenReturn(classLoaderLeakPreventor); + return new ClassLoaderLifeCycle(classLoader, classLoaderLeakPreventorFactory); + } + +} diff --git a/scm-webapp/src/test/java/sonia/scm/plugin/PluginProcessorTest.java b/scm-webapp/src/test/java/sonia/scm/plugin/PluginProcessorTest.java index 694b07e54f..5a38cacd3f 100644 --- a/scm-webapp/src/test/java/sonia/scm/plugin/PluginProcessorTest.java +++ b/scm-webapp/src/test/java/sonia/scm/plugin/PluginProcessorTest.java @@ -42,6 +42,7 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import sonia.scm.boot.ClassLoaderLifeCycle; import static org.hamcrest.Matchers.*; @@ -288,7 +289,7 @@ public class PluginProcessorTest public void setUp() throws IOException { pluginDirectory = temp.newFolder(); - processor = new PluginProcessor(pluginDirectory.toPath()); + processor = new PluginProcessor(ClassLoaderLifeCycle.create(), pluginDirectory.toPath()); } //~--- methods -------------------------------------------------------------- From f747be4331bb3f0060bf9f9b49df233928dd5748 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 19 Jun 2019 11:53:13 +0200 Subject: [PATCH 10/16] create instance counter for EventBus, to improve visibility during restarts --- .../main/java/sonia/scm/event/LegmanScmEventBus.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/event/LegmanScmEventBus.java b/scm-webapp/src/main/java/sonia/scm/event/LegmanScmEventBus.java index fb8fe6d2de..40795a27a6 100644 --- a/scm-webapp/src/main/java/sonia/scm/event/LegmanScmEventBus.java +++ b/scm-webapp/src/main/java/sonia/scm/event/LegmanScmEventBus.java @@ -40,6 +40,8 @@ import com.github.legman.Subscribe; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.concurrent.atomic.AtomicLong; + /** * * @author Sebastian Sdorra @@ -47,8 +49,11 @@ import org.slf4j.LoggerFactory; public class LegmanScmEventBus extends ScmEventBus { + private static final AtomicLong INSTANCE_COUNTER = new AtomicLong(); + + /** Field description */ - private static final String NAME = "ScmEventBus"; + private static final String NAME = "ScmEventBus-%s"; /** * the logger for LegmanScmEventBus @@ -67,8 +72,9 @@ public class LegmanScmEventBus extends ScmEventBus } private EventBus create() { - logger.info("create new event bus {}", NAME); - return new EventBus(NAME); + String name = String.format(NAME, INSTANCE_COUNTER.incrementAndGet()); + logger.info("create new event bus {}", name); + return new EventBus(name); } //~--- methods -------------------------------------------------------------- From 33702e0e9d37d72eb083402a1d2c6724ca8ad340 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 19 Jun 2019 11:53:58 +0200 Subject: [PATCH 11/16] added RestartStrategies to be able to swap the context recreation strategy --- .../scm/boot/BootstrapContextFilter.java | 86 +++++++++--------- .../boot/InjectionContextRestartStrategy.java | 50 +++++++++++ .../java/sonia/scm/boot/RestartStrategy.java | 38 ++++++++ .../InjectionContextRestartStrategyTest.java | 90 +++++++++++++++++++ 4 files changed, 219 insertions(+), 45 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/boot/InjectionContextRestartStrategy.java create mode 100644 scm-webapp/src/main/java/sonia/scm/boot/RestartStrategy.java create mode 100644 scm-webapp/src/test/java/sonia/scm/boot/InjectionContextRestartStrategyTest.java diff --git a/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextFilter.java b/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextFilter.java index 754359ad94..a963e95bb4 100644 --- a/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextFilter.java +++ b/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextFilter.java @@ -37,7 +37,6 @@ import com.github.legman.Subscribe; import com.google.inject.servlet.GuiceFilter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import sonia.scm.event.RecreateEventBusEvent; import sonia.scm.event.ScmEventBus; import javax.servlet.FilterConfig; @@ -50,63 +49,29 @@ import javax.servlet.ServletException; * * @author Sebastian Sdorra */ -public class BootstrapContextFilter extends GuiceFilter -{ +public class BootstrapContextFilter extends GuiceFilter { /** * the logger for BootstrapContextFilter */ - private static final Logger logger = - LoggerFactory.getLogger(BootstrapContextFilter.class); - - //~--- methods -------------------------------------------------------------- + private static final Logger LOG = LoggerFactory.getLogger(BootstrapContextFilter.class); private final BootstrapContextListener listener = new BootstrapContextListener(); - /** - * Restart the whole webapp context. - * - * - * @param event restart event - * - * @throws ServletException - */ - @Subscribe - public void handleRestartEvent(RestartEvent event) throws ServletException - { - logger.warn("received restart event from {} with reason: {}", - event.getCause(), event.getReason()); - - if (filterConfig == null) - { - logger.error("filter config is null, scm-manager is not initialized"); - } - else - { - logger.warn("destroy filter pipeline, because of a received restart event"); - destroy(); - - logger.warn("send recreate eventbus event"); - ScmEventBus.getInstance().post(new RecreateEventBusEvent()); - ScmEventBus.getInstance().register(this); - - logger.warn("reinitialize filter pipeline, because of a received restart event"); - initGuice(); - } - } + /** Field description */ + private FilterConfig filterConfig; @Override - public void init(FilterConfig filterConfig) throws ServletException - { + public void init(FilterConfig filterConfig) throws ServletException { this.filterConfig = filterConfig; initGuice(); - logger.info("register for restart events"); + LOG.info("register for restart events"); ScmEventBus.getInstance().register(this); } - public void initGuice() throws ServletException { + private void initGuice() throws ServletException { super.init(filterConfig); listener.contextInitialized(new ServletContextEvent(filterConfig.getServletContext())); @@ -119,8 +84,39 @@ public class BootstrapContextFilter extends GuiceFilter ServletContextCleaner.cleanup(filterConfig.getServletContext()); } - //~--- fields --------------------------------------------------------------- + /** + * Restart SCM-Manager. + * + * @param event restart event + */ + @Subscribe + public void handleRestartEvent(RestartEvent event) { + LOG.warn("received restart event from {} with reason: {}", + event.getCause(), event.getReason()); + + if (filterConfig == null) { + LOG.error("filter config is null, scm-manager is not initialized"); + } else { + RestartStrategy restartStrategy = RestartStrategy.get(); + restartStrategy.restart(new GuiceInjectionContext()); + } + } + + private class GuiceInjectionContext implements RestartStrategy.InjectionContext { + + @Override + public void initialize() { + try { + BootstrapContextFilter.this.initGuice(); + } catch (ServletException e) { + throw new IllegalStateException("failed to initialize guice", e); + } + } + + @Override + public void destroy() { + BootstrapContextFilter.this.destroy(); + } + } - /** Field description */ - private FilterConfig filterConfig; } diff --git a/scm-webapp/src/main/java/sonia/scm/boot/InjectionContextRestartStrategy.java b/scm-webapp/src/main/java/sonia/scm/boot/InjectionContextRestartStrategy.java new file mode 100644 index 0000000000..29d59d733b --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/boot/InjectionContextRestartStrategy.java @@ -0,0 +1,50 @@ +package sonia.scm.boot; + +import com.google.common.annotations.VisibleForTesting; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import sonia.scm.event.RecreateEventBusEvent; +import sonia.scm.event.ScmEventBus; + +import java.util.concurrent.atomic.AtomicLong; + +/** + * Restart strategy implementation which destroy the injection context and re initialize it. + */ +public class InjectionContextRestartStrategy implements RestartStrategy { + + private static final AtomicLong INSTANCE_COUNTER = new AtomicLong(); + private static final Logger LOG = LoggerFactory.getLogger(InjectionContextRestartStrategy.class); + + private long waitInMs = 250L; + + @VisibleForTesting + void setWaitInMs(long waitInMs) { + this.waitInMs = waitInMs; + } + + @Override + public void restart(InjectionContext context) { + LOG.warn("destroy injection context"); + context.destroy(); + + LOG.warn("send recreate eventbus event"); + ScmEventBus.getInstance().post(new RecreateEventBusEvent()); + + // restart context delayed, to avoid timing problems + new Thread(() -> { + try { + Thread.sleep(waitInMs); + + LOG.warn("reinitialize injection context"); + context.initialize(); + + LOG.debug("re register injection context for events"); + ScmEventBus.getInstance().register(context); + } catch ( Exception ex) { + LOG.error("failed to restart", ex); + } + }, "Delayed-Restart-" + INSTANCE_COUNTER.incrementAndGet()).start(); + + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/boot/RestartStrategy.java b/scm-webapp/src/main/java/sonia/scm/boot/RestartStrategy.java new file mode 100644 index 0000000000..fa1fd052c6 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/boot/RestartStrategy.java @@ -0,0 +1,38 @@ +package sonia.scm.boot; + +/** + * Strategy for restarting SCM-Manager. + */ +public interface RestartStrategy { + + /** + * Context for Injection in SCM-Manager. + */ + interface InjectionContext { + /** + * Initialize the injection context. + */ + void initialize(); + + /** + * Destroys the injection context. + */ + void destroy(); + } + + /** + * Restart SCM-Manager. + * @param context injection context + */ + void restart(InjectionContext context); + + /** + * Returns the configured strategy. + * + * @return configured strategy + */ + static RestartStrategy get() { + return new InjectionContextRestartStrategy(); + } + +} diff --git a/scm-webapp/src/test/java/sonia/scm/boot/InjectionContextRestartStrategyTest.java b/scm-webapp/src/test/java/sonia/scm/boot/InjectionContextRestartStrategyTest.java new file mode 100644 index 0000000000..985ab978b9 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/boot/InjectionContextRestartStrategyTest.java @@ -0,0 +1,90 @@ +package sonia.scm.boot; + +import com.github.legman.Subscribe; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import sonia.scm.event.RecreateEventBusEvent; +import sonia.scm.event.ScmEventBus; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.verify; + +@ExtendWith(MockitoExtension.class) +class InjectionContextRestartStrategyTest { + + @Mock + private RestartStrategy.InjectionContext context; + + private InjectionContextRestartStrategy strategy = new InjectionContextRestartStrategy(); + + @BeforeEach + void setWaitToZero() { + strategy.setWaitInMs(0L); + } + + @Test + void shouldCallDestroyAndInitialize() throws InterruptedException { + strategy.restart(context); + + verify(context).destroy(); + Thread.sleep(50L); + verify(context).initialize(); + } + + @Test + void shouldFireRecreateEventBusEvent() { + Listener listener = new Listener(); + ScmEventBus.getInstance().register(listener); + + strategy.restart(context); + + assertThat(listener.event).isNotNull(); + } + + @Test + void shouldRegisterContextAfterRestart() throws InterruptedException { + TestingInjectionContext ctx = new TestingInjectionContext(); + + strategy.restart(ctx); + + Thread.sleep(50L); + ScmEventBus.getInstance().post("hello event"); + + assertThat(ctx.event).isEqualTo("hello event"); + } + + public static class Listener { + + private RecreateEventBusEvent event; + + @Subscribe(async = false) + public void setEvent(RecreateEventBusEvent event) { + this.event = event; + } + } + + public static class TestingInjectionContext implements RestartStrategy.InjectionContext { + + private volatile String event; + + @Subscribe(async = false) + public void setEvent(String event) { + this.event = event; + } + + @Override + public void initialize() { + + } + + @Override + public void destroy() { + + } + } + +} From cbe983b9f11b22755b110b41297906b3e3a5dfe6 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 20 Jun 2019 14:57:22 +0200 Subject: [PATCH 12/16] log name of eventbus, to make restarts more debuggable --- .../main/java/sonia/scm/event/LegmanScmEventBus.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/event/LegmanScmEventBus.java b/scm-webapp/src/main/java/sonia/scm/event/LegmanScmEventBus.java index 40795a27a6..85f760a71f 100644 --- a/scm-webapp/src/main/java/sonia/scm/event/LegmanScmEventBus.java +++ b/scm-webapp/src/main/java/sonia/scm/event/LegmanScmEventBus.java @@ -63,6 +63,8 @@ public class LegmanScmEventBus extends ScmEventBus //~--- constructors --------------------------------------------------------- + private String name; + /** * Constructs ... * @@ -72,7 +74,7 @@ public class LegmanScmEventBus extends ScmEventBus } private EventBus create() { - String name = String.format(NAME, INSTANCE_COUNTER.incrementAndGet()); + name = String.format(NAME, INSTANCE_COUNTER.incrementAndGet()); logger.info("create new event bus {}", name); return new EventBus(name); } @@ -88,7 +90,7 @@ public class LegmanScmEventBus extends ScmEventBus @Override public void post(Object event) { - logger.debug("post {} to event bus", event); + logger.debug("post {} to event bus {}", event, name); eventBus.post(event); } @@ -102,7 +104,7 @@ public class LegmanScmEventBus extends ScmEventBus @Override public void register(Object object) { - logger.trace("register {} to event bus", object); + logger.trace("register {} to event bus {}", object, name); eventBus.register(object); } @@ -116,7 +118,7 @@ public class LegmanScmEventBus extends ScmEventBus @Override public void unregister(Object object) { - logger.trace("unregister {} from event bus", object); + logger.trace("unregister {} from event bus {}", object, name); try { @@ -130,7 +132,7 @@ public class LegmanScmEventBus extends ScmEventBus @Subscribe(async = false) public void recreateEventBus(RecreateEventBusEvent recreateEventBusEvent) { - logger.info("shutdown event bus executor"); + logger.info("shutdown event bus executor for {}", name); eventBus.shutdown(); eventBus = create(); } From d658a1a662a5345c5b9aa788e65fb373a5bb47cb Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 20 Jun 2019 14:58:32 +0200 Subject: [PATCH 13/16] fix re registration of BootstrapContextFilter after restart --- .../java/sonia/scm/boot/BootstrapContextFilter.java | 13 +++++++------ .../scm/boot/InjectionContextRestartStrategy.java | 2 +- .../boot/InjectionContextRestartStrategyTest.java | 1 - 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextFilter.java b/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextFilter.java index a963e95bb4..a134b44784 100644 --- a/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextFilter.java +++ b/scm-webapp/src/main/java/sonia/scm/boot/BootstrapContextFilter.java @@ -65,14 +65,14 @@ public class BootstrapContextFilter extends GuiceFilter { public void init(FilterConfig filterConfig) throws ServletException { this.filterConfig = filterConfig; - initGuice(); + initializeContext(); + } + + private void initializeContext() throws ServletException { + super.init(filterConfig); LOG.info("register for restart events"); ScmEventBus.getInstance().register(this); - } - - private void initGuice() throws ServletException { - super.init(filterConfig); listener.contextInitialized(new ServletContextEvent(filterConfig.getServletContext())); } @@ -80,6 +80,7 @@ public class BootstrapContextFilter extends GuiceFilter { @Override public void destroy() { super.destroy(); + listener.contextDestroyed(new ServletContextEvent(filterConfig.getServletContext())); ServletContextCleaner.cleanup(filterConfig.getServletContext()); } @@ -107,7 +108,7 @@ public class BootstrapContextFilter extends GuiceFilter { @Override public void initialize() { try { - BootstrapContextFilter.this.initGuice(); + BootstrapContextFilter.this.initializeContext(); } catch (ServletException e) { throw new IllegalStateException("failed to initialize guice", e); } diff --git a/scm-webapp/src/main/java/sonia/scm/boot/InjectionContextRestartStrategy.java b/scm-webapp/src/main/java/sonia/scm/boot/InjectionContextRestartStrategy.java index 29d59d733b..d0b25ba5a9 100644 --- a/scm-webapp/src/main/java/sonia/scm/boot/InjectionContextRestartStrategy.java +++ b/scm-webapp/src/main/java/sonia/scm/boot/InjectionContextRestartStrategy.java @@ -39,7 +39,7 @@ public class InjectionContextRestartStrategy implements RestartStrategy { LOG.warn("reinitialize injection context"); context.initialize(); - LOG.debug("re register injection context for events"); + LOG.debug("register injection context on new eventbus"); ScmEventBus.getInstance().register(context); } catch ( Exception ex) { LOG.error("failed to restart", ex); diff --git a/scm-webapp/src/test/java/sonia/scm/boot/InjectionContextRestartStrategyTest.java b/scm-webapp/src/test/java/sonia/scm/boot/InjectionContextRestartStrategyTest.java index 985ab978b9..81e7faa6d5 100644 --- a/scm-webapp/src/test/java/sonia/scm/boot/InjectionContextRestartStrategyTest.java +++ b/scm-webapp/src/test/java/sonia/scm/boot/InjectionContextRestartStrategyTest.java @@ -1,7 +1,6 @@ package sonia.scm.boot; import com.github.legman.Subscribe; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; From eec4b282e69605f0815329598121670f16193a5a Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 20 Jun 2019 15:09:12 +0200 Subject: [PATCH 14/16] remove MBeanCleanUp step to avoid exception on restart --- .../src/main/java/sonia/scm/boot/ClassLoaderLifeCycle.java | 2 ++ scm-webapp/src/main/java/sonia/scm/boot/LoggingAdapter.java | 1 + 2 files changed, 3 insertions(+) diff --git a/scm-webapp/src/main/java/sonia/scm/boot/ClassLoaderLifeCycle.java b/scm-webapp/src/main/java/sonia/scm/boot/ClassLoaderLifeCycle.java index 648197b7e4..be1d4d3653 100644 --- a/scm-webapp/src/main/java/sonia/scm/boot/ClassLoaderLifeCycle.java +++ b/scm-webapp/src/main/java/sonia/scm/boot/ClassLoaderLifeCycle.java @@ -5,6 +5,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import se.jiderhamn.classloader.leak.prevention.ClassLoaderLeakPreventor; import se.jiderhamn.classloader.leak.prevention.ClassLoaderLeakPreventorFactory; +import se.jiderhamn.classloader.leak.prevention.cleanup.MBeanCleanUp; import sonia.scm.plugin.ChildFirstPluginClassLoader; import sonia.scm.plugin.DefaultPluginClassLoader; @@ -36,6 +37,7 @@ public final class ClassLoaderLifeCycle { public static ClassLoaderLifeCycle create() { ClassLoaderLeakPreventorFactory classLoaderLeakPreventorFactory = new ClassLoaderLeakPreventorFactory(); classLoaderLeakPreventorFactory.setLogger(new LoggingAdapter()); + classLoaderLeakPreventorFactory.removeCleanUp(MBeanCleanUp.class); return new ClassLoaderLifeCycle(Thread.currentThread().getContextClassLoader(), classLoaderLeakPreventorFactory); } diff --git a/scm-webapp/src/main/java/sonia/scm/boot/LoggingAdapter.java b/scm-webapp/src/main/java/sonia/scm/boot/LoggingAdapter.java index 5c760f3081..cc7f6befea 100644 --- a/scm-webapp/src/main/java/sonia/scm/boot/LoggingAdapter.java +++ b/scm-webapp/src/main/java/sonia/scm/boot/LoggingAdapter.java @@ -9,6 +9,7 @@ import se.jiderhamn.classloader.leak.prevention.ClassLoaderLeakPreventor; */ public class LoggingAdapter implements se.jiderhamn.classloader.leak.prevention.Logger { + @SuppressWarnings("squid:S3416") // suppress "loggers should be named for their enclosing classes" rule private static final Logger LOG = LoggerFactory.getLogger(ClassLoaderLeakPreventor.class); @Override From 1e586b020c5880c58981213a001adce1593dd192 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 24 Jun 2019 17:51:38 +0200 Subject: [PATCH 15/16] Make sure task is not executed after is has cancelled himself --- .../sonia/scm/schedule/CronExpression.java | 2 +- .../java/sonia/scm/schedule/CronTask.java | 3 +- .../java/sonia/scm/schedule/CronTaskTest.java | 40 ++++++++----------- 3 files changed, 20 insertions(+), 25 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/schedule/CronExpression.java b/scm-webapp/src/main/java/sonia/scm/schedule/CronExpression.java index fbbff51f57..116f9bff82 100644 --- a/scm-webapp/src/main/java/sonia/scm/schedule/CronExpression.java +++ b/scm-webapp/src/main/java/sonia/scm/schedule/CronExpression.java @@ -30,7 +30,7 @@ final class CronExpression { boolean shouldRun(ZonedDateTime time) { ZonedDateTime now = ZonedDateTime.now(clock); - return time.isBefore(now); + return time.isBefore(now) || time.isEqual(now); } Optional calculateNextRun() { diff --git a/scm-webapp/src/main/java/sonia/scm/schedule/CronTask.java b/scm-webapp/src/main/java/sonia/scm/schedule/CronTask.java index 0e8c636184..e1a9ded268 100644 --- a/scm-webapp/src/main/java/sonia/scm/schedule/CronTask.java +++ b/scm-webapp/src/main/java/sonia/scm/schedule/CronTask.java @@ -32,7 +32,7 @@ class CronTask implements Task, Runnable { @Override public synchronized void run() { - if (expression.shouldRun(nextRun)) { + if (hasNextRun() && expression.shouldRun(nextRun)) { LOG.debug("execute task {}, because of matching expression {}", name, expression); runnable.run(); Optional next = expression.calculateNextRun(); @@ -40,6 +40,7 @@ class CronTask implements Task, Runnable { nextRun = next.get(); } else { LOG.debug("cancel task {}, because expression {} has no next execution", name, expression); + nextRun = null; cancel(); } } else { diff --git a/scm-webapp/src/test/java/sonia/scm/schedule/CronTaskTest.java b/scm-webapp/src/test/java/sonia/scm/schedule/CronTaskTest.java index 7cfbba29e3..783ec61d0b 100644 --- a/scm-webapp/src/test/java/sonia/scm/schedule/CronTaskTest.java +++ b/scm-webapp/src/test/java/sonia/scm/schedule/CronTaskTest.java @@ -53,7 +53,7 @@ class CronTaskTest { @Test void shouldCancelWithoutNextRun() { ZonedDateTime time = ZonedDateTime.now(); - when(expression.calculateNextRun()).thenAnswer(new FirstTimeAnswer(Optional.of(time), Optional.empty())); + when(expression.calculateNextRun()).thenReturn(Optional.of(time), Optional.empty()); when(expression.shouldRun(time)).thenReturn(true); CronTask task = task(); @@ -64,32 +64,26 @@ class CronTaskTest { verify(future).cancel(false); } + @Test + void shouldNotRunAfterCancelHasBeenCalledIfRunIsCalledAgain() { + ZonedDateTime time = ZonedDateTime.now(); + when(expression.calculateNextRun()).thenReturn(Optional.of(time), Optional.empty()); + when(expression.shouldRun(time)).thenReturn(true); + + CronTask task = task(); + task.setFuture(future); + + task.run(); + task.run(); + + verify(future).cancel(false); + verify(runnable).run(); + } + @Test void shouldNotRun() { task().run(); verify(runnable, never()).run(); } - - private static class FirstTimeAnswer implements Answer { - - private boolean first = true; - private final Object answer; - private final Object secondAnswer; - - FirstTimeAnswer(Object answer, Object secondAnswer) { - this.answer = answer; - this.secondAnswer = secondAnswer; - } - - @Override - public Object answer(InvocationOnMock invocation) { - if (first) { - first = false; - return answer; - } - return secondAnswer; - } - } - } From 9a99c841df8ba66b842be20a088b0f703e42506f Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Tue, 25 Jun 2019 07:30:02 +0000 Subject: [PATCH 16/16] Close branch feature/restart_context