Skip to content

Commit

Permalink
Aggressively clear the fields of the QuarkusClassLoader on close
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@gsmet
gsmet committed Jul 2, 2024
1 parent 69ebf8a commit 1493f97
Showing 1 changed file with 82 additions and 19 deletions.
101 changes: 82 additions & 19 deletions ...ts/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.";

Expand Down Expand Up @@ -103,7 +107,7 @@ public static boolean isResourcePresentAtRuntime(String resourcePath) {
private final List<ClassLoaderEventListener> classLoaderEventListeners;

/**
* The element that holds resettable in-memory classses.
* The element that holds resettable in-memory classes.
* <p>
* 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
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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;
}
Expand Down Expand Up @@ -198,6 +207,8 @@ private boolean parentFirst(String name, ClassLoaderState state) {
}

public void reset(Map<String, byte[]> generatedResources, Map<String, byte[]> transformedClasses) {
ensureOpen();

if (resettableElement == null) {
throw new IllegalStateException("Classloader is not resettable");
}
Expand All @@ -210,10 +221,14 @@ public void reset(Map<String, byte[]> generatedResources, Map<String, byte[]> tr

@Override
public Enumeration<URL> getResources(String unsanitisedName) throws IOException {
ensureOpen();

return getResources(unsanitisedName, false);
}

public Enumeration<URL> getResources(String unsanitisedName, boolean parentAlreadyFoundResources) throws IOException {
ensureOpen();

for (ClassLoaderEventListener l : classLoaderEventListeners) {
l.enumeratingResourceURLs(unsanitisedName, this.name);
}
Expand Down Expand Up @@ -244,7 +259,7 @@ public Enumeration<URL> 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<URL> resources = new LinkedHashSet<>();
ClassPathElement[] providers = state.loadableResources.get(name);
if (providers != null) {
Expand Down Expand Up @@ -356,6 +371,8 @@ private ClassLoaderState getState() {

@Override
public URL getResource(String unsanitisedName) {
ensureOpen();

for (ClassLoaderEventListener l : classLoaderEventListeners) {
l.gettingURLFromResource(unsanitisedName, this.name);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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) {
Expand All @@ -463,21 +484,29 @@ protected Class<?> findClass(String moduleName, String name) {
}

protected URL findResource(String name) {
ensureOpen();

return getResource(name);
}

@Override
protected Enumeration<URL> 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);
}
Expand Down Expand Up @@ -574,10 +603,14 @@ private String getPackageNameFromClassName(String className) {
}

public List<ClassPathElement> getElementsWithResource(String name) {
ensureOpen();

return getElementsWithResource(name, false);
}

public List<ClassPathElement> getElementsWithResource(String name, boolean localOnly) {
ensureOpen();

List<ClassPathElement> ret = new ArrayList<>();
if (parent instanceof QuarkusClassLoader && !localOnly) {
ret.addAll(((QuarkusClassLoader) parent).getElementsWithResource(name));
Expand All @@ -591,6 +624,8 @@ public List<ClassPathElement> getElementsWithResource(String name, boolean local
}

public List<String> getLocalClassNames() {
ensureOpen();

List<String> ret = new ArrayList<>();
for (String name : getState().loadableResources.keySet()) {
if (name.endsWith(".class")) {
Expand All @@ -602,10 +637,14 @@ public List<String> 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);
}
Expand All @@ -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<Runnable> tasks;
synchronized (closeTasks) {
tasks = new ArrayList<>(closeTasks);
Expand All @@ -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<ClassPathElement> 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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 1493f97

Please sign in to comment.