From 1493f97501cdb0e5c4a39f6d85cb736cc199c5a1 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Wed, 12 Jun 2024 19:31:28 +0200 Subject: [PATCH] Aggressively clear the fields of the QuarkusClassLoader on close We know that the class loaders can leak, for instance due to long lived resources that span the boundaries of a test so we try to limit the effects by clearing the fields from the class loader and especially all the ClassPathElements. Note that for now, I didn't nullify the parent field that points to the parent CL but I wonder if we should do it. We also add some logging to debug the lifecycle of the class loader. We can't easily log things in the close() method so things are not as clean as they could be. That's the reason why we are using a system property to enable the logging. You can log the constructor and close() calls by enabling debug logging for category `io.quarkus.bootstrap.classloading.QuarkusClassLoader.lifecycle`. You can log late accesses to closed class loaders by passing `-Dquarkus-log-access-to-closed-class-loaders` to your build command. --- .../classloading/QuarkusClassLoader.java | 101 ++++++++++++++---- 1 file changed, 82 insertions(+), 19 deletions(-) diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java index fdac726f9b609..a7a52a5566505 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java @@ -35,6 +35,10 @@ */ public class QuarkusClassLoader extends ClassLoader implements Closeable { private static final Logger log = Logger.getLogger(QuarkusClassLoader.class); + private static final Logger lifecycleLog = Logger.getLogger(QuarkusClassLoader.class.getName() + ".lifecycle"); + private static final boolean LOG_ACCESS_TO_CLOSED_CLASS_LOADERS = Boolean + .getBoolean("quarkus-log-access-to-closed-class-loaders"); + protected static final String META_INF_SERVICES = "META-INF/services/"; protected static final String JAVA = "java."; @@ -103,7 +107,7 @@ public static boolean isResourcePresentAtRuntime(String resourcePath) { private final List classLoaderEventListeners; /** - * The element that holds resettable in-memory classses. + * The element that holds resettable in-memory classes. *

* A reset occurs when new transformers and in-memory classes are added to a ClassLoader. It happens after each * start in dev mode, however in general the reset resources will be the same. There are some cases where this is @@ -131,7 +135,8 @@ public static boolean isResourcePresentAtRuntime(String resourcePath) { PLATFORM_CLASS_LOADER = cl; } - private boolean closed; + private volatile boolean closing; + private volatile boolean closed; private volatile boolean driverLoaded; private QuarkusClassLoader(Builder builder) { @@ -151,6 +156,8 @@ private QuarkusClassLoader(Builder builder) { this.classLoaderEventListeners = builder.classLoaderEventListeners.isEmpty() ? Collections.emptyList() : builder.classLoaderEventListeners; setDefaultAssertionStatus(builder.assertionsEnabled); + + lifecycleLog.debugf(new RuntimeException("Created to log a stacktrace"), "Creating class loader %s", this); } public static Builder builder(String name, ClassLoader parent, boolean parentFirst) { @@ -171,6 +178,8 @@ private String sanitizeName(String name) { * Returns true if the supplied class is a class that would be loaded parent-first */ public boolean isParentFirst(String name) { + ensureOpen(); + if (name.startsWith(JAVA)) { return true; } @@ -198,6 +207,8 @@ private boolean parentFirst(String name, ClassLoaderState state) { } public void reset(Map generatedResources, Map transformedClasses) { + ensureOpen(); + if (resettableElement == null) { throw new IllegalStateException("Classloader is not resettable"); } @@ -210,10 +221,14 @@ public void reset(Map generatedResources, Map tr @Override public Enumeration getResources(String unsanitisedName) throws IOException { + ensureOpen(); + return getResources(unsanitisedName, false); } public Enumeration getResources(String unsanitisedName, boolean parentAlreadyFoundResources) throws IOException { + ensureOpen(); + for (ClassLoaderEventListener l : classLoaderEventListeners) { l.enumeratingResourceURLs(unsanitisedName, this.name); } @@ -244,7 +259,7 @@ public Enumeration getResources(String unsanitisedName, boolean parentAlrea } } //TODO: in theory resources could have been added in dev mode - //but I don't thing this really matters for this code path + //but I don't think this really matters for this code path Set resources = new LinkedHashSet<>(); ClassPathElement[] providers = state.loadableResources.get(name); if (providers != null) { @@ -356,6 +371,8 @@ private ClassLoaderState getState() { @Override public URL getResource(String unsanitisedName) { + ensureOpen(); + for (ClassLoaderEventListener l : classLoaderEventListeners) { l.gettingURLFromResource(unsanitisedName, this.name); } @@ -404,6 +421,8 @@ public URL getResource(String unsanitisedName) { @Override public InputStream getResourceAsStream(String unsanitisedName) { + ensureOpen(); + for (ClassLoaderEventListener l : classLoaderEventListeners) { l.openResourceStream(unsanitisedName, this.name); } @@ -455,6 +474,8 @@ public InputStream getResourceAsStream(String unsanitisedName) { */ @Override protected Class findClass(String moduleName, String name) { + ensureOpen(); + try { return loadClass(name, false); } catch (ClassNotFoundException e) { @@ -463,21 +484,29 @@ protected Class findClass(String moduleName, String name) { } protected URL findResource(String name) { + ensureOpen(); + return getResource(name); } @Override protected Enumeration findResources(String name) throws IOException { + ensureOpen(); + return getResources(name); } @Override public Class loadClass(String name) throws ClassNotFoundException { + ensureOpen(); + return loadClass(name, false); } @Override protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + ensureOpen(); + for (ClassLoaderEventListener l : classLoaderEventListeners) { l.loadClass(name, this.name); } @@ -574,10 +603,14 @@ private String getPackageNameFromClassName(String className) { } public List getElementsWithResource(String name) { + ensureOpen(); + return getElementsWithResource(name, false); } public List getElementsWithResource(String name, boolean localOnly) { + ensureOpen(); + List ret = new ArrayList<>(); if (parent instanceof QuarkusClassLoader && !localOnly) { ret.addAll(((QuarkusClassLoader) parent).getElementsWithResource(name)); @@ -591,6 +624,8 @@ public List getElementsWithResource(String name, boolean local } public List getLocalClassNames() { + ensureOpen(); + List ret = new ArrayList<>(); for (String name : getState().loadableResources.keySet()) { if (name.endsWith(".class")) { @@ -602,10 +637,14 @@ public List getLocalClassNames() { } public Class visibleDefineClass(String name, byte[] b, int off, int len) throws ClassFormatError { + ensureOpen(); + return super.defineClass(name, b, off, len); } public void addCloseTask(Runnable task) { + ensureOpen(); + synchronized (closeTasks) { closeTasks.add(task); } @@ -614,11 +653,14 @@ public void addCloseTask(Runnable task) { @Override public void close() { synchronized (this) { - if (closed) { + if (closing) { return; } - closed = true; + closing = true; } + + lifecycleLog.debugf(new RuntimeException("Created to log a stacktrace"), "Closing class loader %s", this); + List tasks; synchronized (closeTasks) { tasks = new ArrayList<>(closeTasks); @@ -642,17 +684,26 @@ public void close() { log.debug("Failed to clean up DB drivers"); } } - for (ClassPathElement element : elements) { - //note that this is a 'soft' close - //all resources are closed, however the CL can still be used - //but after close no resources will be held past the scope of an operation - try (ClassPathElement ignored = element) { - //the close() operation is implied by the try-with syntax - } catch (Exception e) { - log.error("Failed to close " + element, e); - } - } - for (ClassPathElement element : bannedElements) { + closeClassPathElements(elements); + closeClassPathElements(bannedElements); + closeClassPathElements(parentFirstElements); + closeClassPathElements(lesserPriorityElements); + + protectionDomains.clear(); + definedPackages.clear(); + resettableElement = null; + transformedClasses = null; + state.clear(); + closeTasks.clear(); + classLoaderEventListeners.clear(); + + ResourceBundle.clearCache(this); + + closed = true; + } + + private static void closeClassPathElements(List classPathElements) { + for (ClassPathElement element : classPathElements) { //note that this is a 'soft' close //all resources are closed, however the CL can still be used //but after close no resources will be held past the scope of an operation @@ -662,12 +713,19 @@ public void close() { log.error("Failed to close " + element, e); } } - ResourceBundle.clearCache(this); - + classPathElements.clear(); } public boolean isClosed() { - return closed; + return closing; + } + + private void ensureOpen() { + if (LOG_ACCESS_TO_CLOSED_CLASS_LOADERS && closed) { + // we do not use a logger as it might require some class loading + System.out.println("Class loader " + this + " has been closed and may not be accessed anymore"); + Thread.dumpStack(); + } } @Override @@ -841,6 +899,11 @@ static final class ClassLoaderState { this.bannedResources = bannedResources; this.parentFirstResources = parentFirstResources; } + + void clear() { + // when the CL is closed, we make sure the resources are not loadable anymore + loadableResources.clear(); + } } @Override