use ClassLoaderLeakPreventor to reduce ClassLoaderLeaks of plugins

scm-webapp/pom.xml

@@ -276,6 +276,14 @@
       <version>1.20</version>
     </dependency>
 
+    <!-- class loader leak prevention -->
+
+    <dependency>
+      <groupId>se.jiderhamn.classloader-leak-prevention</groupId>
+      <artifactId>classloader-leak-prevention-core</artifactId>
+      <version>2.7.0</version>
+    </dependency>
+
     <!-- test scope -->
 
     <dependency>

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() {

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);
+  }
+}

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<PluginWrapper> 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<PluginWrapper> plugins, PluginLoader pluginLoader, Injector bootstrapInjector) {

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<ClassLoaderAndPreventor> classLoaders = new ArrayDeque<>();
+
+  private final ClassLoaderLeakPreventorFactory classLoaderLeakPreventorFactory;
+  private final ClassLoader webappClassLoader;
+
+  private ClassLoader bootstrapClassLoader;
+  private UnaryOperator<ClassLoader> 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<ClassLoader> 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);
+        }
+      }
+    }
+  }
+}

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);
+  }
+}

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

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

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;

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<PluginWrapper> collectPlugins(ClassLoader classLoader,
+  public static Set<PluginWrapper> 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());
   }
 
   /**

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);
+  }
+
+}

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