Skip to content

Commit

Permalink
Remove shutdown hook entirely (#376)
Browse files Browse the repository at this point in the history
  • Loading branch information
lukehutch committed Oct 20, 2019
1 parent ace3707 commit 05e33f4
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 79 deletions.
14 changes: 0 additions & 14 deletions src/main/java/io/github/classgraph/ClassGraph.java
Expand Up @@ -87,20 +87,6 @@ public ClassGraph() {
ScanResult.init();
}

/**
* Disable the adding of a shutdown hook, which closes all files and direct byte buffers on shutdown, enabling
* clean shutdown even if the user forgets to call {@link ScanResult#close()} on a {@link ScanResult}. This
* static method must be called before the first call to the {@link ClassGraph#ClassGraph()} constructor, since
* the first call to the constructor adds the shutdown hook if this has not been disabled.
*
* <p>
* The shutdown hook should be disabled in environments where apps may be loaded and unloaded many times
* throughout the lifetime of the VM, such as web application servers.
*/
public static void disableShutdownHook() {
ScanResult.enableShutdownHook.set(false);
}

/**
* Get the version number of ClassGraph.
*
Expand Down
71 changes: 23 additions & 48 deletions src/main/java/io/github/classgraph/ScanResult.java
Expand Up @@ -132,9 +132,6 @@ public final class ScanResult implements Closeable, AutoCloseable {
/** If true, ScanResult#staticInit() has been run. */
private static final AtomicBoolean initialized = new AtomicBoolean(false);

/** If true, add a shutdown hook to close all files and direct byte buffers on shutdown. */
static final AtomicBoolean enableShutdownHook = new AtomicBoolean(true);

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

/** The current serialization format. */
Expand Down Expand Up @@ -205,46 +202,15 @@ public SerializationFormat(final String serializationFormatStr, final ScanSpec s
static void init() {
if (!initialized.getAndSet(true)) {
// 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).
// to be loaded to close resources are already loaded and cached. This was originally for use in
// a shutdown hook (#331), which has now been removed, but it is probably still a good idea to
// ensure that classes needed to unmap DirectByteBuffer instances are available at init.
// 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.
FileUtils.closeDirectByteBuffer(ByteBuffer.allocateDirect(32), /* log = */ null);
}
}

/**
* Add a shutdown hook, if this is the first time ClassGraph has been run, and if the shutdown hook is enabled
* for the current context classloader.
*/
static void addShutdownHook() {
if (enableShutdownHook.get()) {
// Add runtime shutdown hook to remove temporary files on Ctrl-C or System.exit().
Runtime.getRuntime().addShutdownHook(new Thread(new Runnable() {
@Override
public void run() {
// Close all open/mapped DirectByteBuffers on shutdown
ScanResult.closeAll();
}
}, "ClassGraph-shutdown-hook"));
}
}

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

// -------------------------------------------------------------------------------------------------------------
// Constructor

Expand Down Expand Up @@ -327,13 +293,8 @@ public static void closeAll() {
this.classGraphClassLoader = new ClassGraphClassLoader(this);

// Provide the shutdown hook with a weak reference to this ScanResult
if (nonClosedWeakReferences != null) {
this.weakReference = new WeakReference<>(this);
nonClosedWeakReferences.add(this.weakReference);
} else {
// Should not happen
throw new RuntimeException("nonClosedWeakReferences should not be null");
}
this.weakReference = new WeakReference<>(this);
nonClosedWeakReferences.add(this.weakReference);
}

/** Index {@link Resource} and {@link ClassInfo} objects. */
Expand Down Expand Up @@ -1390,6 +1351,7 @@ public boolean isObtainedFromDeserialization() {
@Override
public void close() {
if (!closed.getAndSet(true)) {
nonClosedWeakReferences.remove(weakReference);
if (classpathOrder != null) {
classpathOrder.clear();
classpathOrder = null;
Expand Down Expand Up @@ -1430,14 +1392,27 @@ public void close() {
}
classGraphClassLoader = null;
classLoaderOrderRespectingParentDelegation = null;
// Remove WeakReference to this ScanResult, so shutdown hook does not try to close this
if (nonClosedWeakReferences != null) {
nonClosedWeakReferences.remove(weakReference);
}
// Flush log on exit, in case additional log entries were generated after scan() completed
if (topLevelLog != null) {
topLevelLog.flush();
}
}
}

/**
* Close all {@link ScanResult} instances that have not yet been closed. Note that this will close all open
* {@link ScanResult} instances for any class that uses the classloader that the {@link ScanResult} class is
* cached in -- so if you call this method, you need to ensure that the lifecycle of the classloader matches the
* lifecycle of your application, or that two concurrent applications don't share the same classloader,
* otherwise one application might close another application's {@link ScanResult} instances while they are still
* in use.
*/
public static void closeAll() {
for (final WeakReference<ScanResult> nonClosedWeakReference : new ArrayList<>(nonClosedWeakReferences)) {
final ScanResult scanResult = nonClosedWeakReference.get();
if (scanResult != null) {
scanResult.close();
}
}
}
}
3 changes: 0 additions & 3 deletions src/main/java/io/github/classgraph/Scanner.java
Expand Up @@ -213,9 +213,6 @@ class Scanner implements Callable<ScanResult> {
}
}
}

// Add a shutdown hook if necessary
ScanResult.addShutdownHook();
}

// -------------------------------------------------------------------------------------------------------------
Expand Down
Expand Up @@ -38,12 +38,11 @@
import io.github.classgraph.ScanResult;

/**
* Register a {@link WebListener} to respond to servlet context shutdown (#376). This creates classfile references
* to javax.servlet classes, however this class is never actually referenced by any other class in ClassGraph (it is
* only included in the classpath so that the servlet container can locate it using the {@link WebListener}
* annotation). Therefore ClassGraph has only a compile-time ("provides"-scoped) dependency on the servlet container
* to enable the container to find this class, but a ClassNotFound exception should not be thrown by anything else
* that uses ClassGraph.
* Register a {@link WebListener} to respond to servlet context shutdown by closing any remaining open ScanResult
* instances (#376). This creates classfile references to javax.servlet classes, however this class is never
* referenced by any other class in ClassGraph (it is only included in the classpath so that the servlet container
* can locate it using the {@link WebListener} annotation). Therefore ClassGraph has only a compile-time
* ("provides"-scoped) dependency on the servlet container to enable the container to find this class.
*/
@WebListener
public class ServletLifeCycleListener implements ServletContextListener {
Expand All @@ -58,8 +57,7 @@ public class ServletLifeCycleListener implements ServletContextListener {
*/
@Override
public void contextInitialized(final ServletContextEvent event) {
log.info("Servlet container initialized -- disabling ClassGraph shutdown hook");
ClassGraph.disableShutdownHook();
log.info("Servlet context initialized");
}

/**
Expand Down
Expand Up @@ -35,12 +35,11 @@
import org.springframework.context.annotation.Configuration;

/**
* Register a {@link ServletContextListener} to respond to Spring context shutdown (#376). This creates classfile
* references to Spring and servlet classes, however this class is never actually referenced by any other class in
* ClassGraph (it is only included in the classpath so that Spring can locate it using the {@link Configuration}
* annotation). Therefore ClassGraph has only a compile-time ("provides"-scoped) dependency on Spring to enable
* Spring to find this class, but a ClassNotFound exception should not be thrown by anything else that uses
* ClassGraph.
* Register a {@link ServletContextListener} to respond to Spring context shutdown by closing any remaining open
* ScanResult instances (#376). This creates classfile references to Spring and servlet classes, however this class
* is never actually referenced by any other class in ClassGraph (it is only included in the classpath so that
* Spring can locate it using the {@link Configuration} annotation). Therefore ClassGraph has only a compile-time
* ("provides"-scoped) dependency on Spring to enable Spring to find this class.
*/
@Configuration
public class SpringLifeCycleListener {
Expand Down

0 comments on commit 05e33f4

Please sign in to comment.