Skip to content

Commit

Permalink
Ensure that context classloader ref is not held by shutdown hook (#376).
Browse files Browse the repository at this point in the history
  • Loading branch information
lukehutch committed Oct 18, 2019
1 parent f5b4fe9 commit d38ff9f
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 41 deletions.
2 changes: 1 addition & 1 deletion src/main/java/io/github/classgraph/ClassGraph.java
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public class ClassGraph {
static {
ScanResult.initShutdownHook();
}

// -------------------------------------------------------------------------------------------------------------

/** Construct a ClassGraph instance. */
Expand Down
81 changes: 41 additions & 40 deletions src/main/java/io/github/classgraph/ScanResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -196,56 +196,37 @@ public SerializationFormat(final String serializationFormatStr, final ScanSpec s
// Shutdown hook init code

static void initShutdownHook() {
final Thread hookThread = new Thread() {
@Override
public void run() {
// Close all ScanResult instances that have not yet been closed
if (nonClosedWeakReferences != null) {
for (final WeakReference<ScanResult> nonClosedWeakReference : new ArrayList<>(
nonClosedWeakReferences)) {
final ScanResult scanResult = nonClosedWeakReference.get();
if (scanResult != null) {
scanResult.close();
}
nonClosedWeakReferences.remove(nonClosedWeakReference);
}
}
}
};

// Set thread classloader to system classloader, so that a reference is not held to the caller's
// For shutdown hook, use system classloader, so that a reference is not held to the caller's
// classloader, e.g. servlet container classloader (#376)
final ClassLoader systemClassLoader = MappedByteBuffer.class.getClassLoader();
hookThread.setContextClassLoader(systemClassLoader);

// Pre-load non-system classes necessary for calling scanResult.close(), so that classes that need
// to be loaded to close resources are already loaded and cached. Otherwise, the classloader may be
// closed by its own shutdown hook before ClassGraph's shutdown hook can run, and classloading can
// fail, which will throw an exception and leave resources open (#331).
// We achieve this by mmap'ing a file and then closing it, since the only problematic classes are
// the PriviledgedAction anonymous inner classes used by FileUtils::closeDirectByteBuffer.
final Semaphore wait = new Semaphore(0);
final Thread preloadClassesThread = new Thread() {
@Override
public void run() {
// Create the new ConcurrentHashMap from the system classloader
nonClosedWeakReferences = Collections
.newSetFromMap(new ConcurrentHashMap<WeakReference<ScanResult>, Boolean>());
try {
// Warm up the classloader, caching the classes necessary to close direct byte buffers
FileUtils.closeDirectByteBuffer(ByteBuffer.allocateDirect(32), /* log = */ null);
} catch (final Exception e) {
// Should not happen
throw new RuntimeException(e);
// Run this thread with the same context classloader as the shutdown hook thread (the system classloader).
try (AutoCloseableExecutorService executorService = new AutoCloseableExecutorService(1,
systemClassLoader)) {
final Semaphore wait = new Semaphore(0);
executorService.execute(new Runnable() {
@Override
public void run() {
// Create the new ConcurrentHashMap from the system classloader
nonClosedWeakReferences = Collections
.newSetFromMap(new ConcurrentHashMap<WeakReference<ScanResult>, Boolean>());
try {
// Warm up the classloader, caching the classes necessary to close direct byte buffers
FileUtils.closeDirectByteBuffer(ByteBuffer.allocateDirect(32), /* log = */ null);
} catch (final Exception e) {
// Should not happen
throw new RuntimeException(e);
}
// Allow code below to continue
wait.release();
}
// Allow code below to continue
wait.release();
}
};
// Run this thread with the same context classloader as the shutdown hook thread (the system classloader)
preloadClassesThread.setContextClassLoader(systemClassLoader);
try (AutoCloseableExecutorService executorService = new AutoCloseableExecutorService(1)) {
executorService.execute(preloadClassesThread);
});
try {
// Wait for preloadClassesThread to finish running
wait.acquire();
Expand All @@ -255,6 +236,26 @@ public void run() {
}
}

// Create shutdown hook thread to close all open/mapped DirectByteBuffers on shutdown
final Thread hookThread = new Thread() {
@Override
public void run() {
// Close all ScanResult instances that have not yet been closed
if (nonClosedWeakReferences != null) {
for (final WeakReference<ScanResult> nonClosedWeakReference : new ArrayList<>(
nonClosedWeakReferences)) {
final ScanResult scanResult = nonClosedWeakReference.get();
if (scanResult != null) {
scanResult.close();
}
nonClosedWeakReferences.remove(nonClosedWeakReference);
}
}
}
};
// Set shutdown hook thread classloader to system classloader, dropping ref to context classloader
hookThread.setContextClassLoader(systemClassLoader);

// Add runtime shutdown hook to remove temporary files on Ctrl-C or System.exit().
Runtime.getRuntime().addShutdownHook(hookThread);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,19 @@ public AutoCloseableExecutorService(final int numThreads) {
new SimpleThreadFactory("ClassGraph-worker-", true));
}

/**
* A ThreadPoolExecutor that can be used in a try-with-resources block.
*
* @param numThreads
* The number of threads to allocate.
* @param threadContextClassLoader
* The context classloader to use for new threads, or null to use the caller's classloader
*/
public AutoCloseableExecutorService(final int numThreads, final ClassLoader threadContextClassLoader) {
super(numThreads, numThreads, 0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<Runnable>(),
new SimpleThreadFactory("ClassGraph-worker-", true, threadContextClassLoader));
}

/**
* Catch exceptions from both submit() and execute(), and call {@link InterruptionChecker#interrupt()} to
* interrupt all threads.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ class SimpleThreadFactory implements java.util.concurrent.ThreadFactory {
/** Whether to set daemon mode. */
private final boolean daemon;

/** The context classloader to use for new threads, or null to use the caller's classloader. */
private final ClassLoader threadContextClassLoader;

/**
* Constructor.
*
Expand All @@ -57,6 +60,24 @@ class SimpleThreadFactory implements java.util.concurrent.ThreadFactory {
SimpleThreadFactory(final String threadNamePrefix, final boolean daemon) {
this.threadNamePrefix = threadNamePrefix;
this.daemon = daemon;
this.threadContextClassLoader = null;
}

/**
* Constructor.
*
* @param threadNamePrefix
* prefix for created threads.
* @param daemon
* create daemon threads?
* @param threadContextClassLoader
* The context classloader to use for new threads, or null to use the caller's classloader
*/
SimpleThreadFactory(final String threadNamePrefix, final boolean daemon,
final ClassLoader threadContextClassLoader) {
this.threadNamePrefix = threadNamePrefix;
this.daemon = daemon;
this.threadContextClassLoader = threadContextClassLoader;
}

/* (non-Javadoc)
Expand All @@ -65,6 +86,9 @@ class SimpleThreadFactory implements java.util.concurrent.ThreadFactory {
@Override
public Thread newThread(final Runnable r) {
final Thread t = new Thread(r, threadNamePrefix + threadIdx.getAndIncrement());
if (threadContextClassLoader != null) {
t.setContextClassLoader(threadContextClassLoader);
}
t.setDaemon(daemon);
return t;
}
Expand Down

0 comments on commit d38ff9f

Please sign in to comment.