From 4c81b9b5a6d186dbe77b691ddb25717e4cceaba0 Mon Sep 17 00:00:00 2001 From: gtoison Date: Thu, 21 Apr 2022 15:17:53 +0200 Subject: [PATCH] fix: let Plugin implement AutoCloseable When a custom plugin is loaded from a .jar file PluginLoader creates URLClassLoader and never closes it. This leaves a lock (a RandomAccessFile) on the .jar file and it cannot be deleted or renamed until the JVM process stops. Keep track of the URLClassLoader created by the PluginLoader so we can close them when we are done with the Plugin --- CHANGELOG.md | 1 + .../main/java/edu/umd/cs/findbugs/Plugin.java | 12 +++- .../edu/umd/cs/findbugs/PluginLoader.java | 57 +++++++++++++++++-- 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2c6618c11c..c7cf3deff1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2. ## Unreleased - 2022-??-?? ### Changed - Updated documentation by adding parenthesis `()` to the negative odd check message ([#1995](https://github.com/spotbugs/spotbugs/issues/1995)) +- Let the Plugin class implement AutoCloseable so we can release the .jar file ([#2024](https://github.com/spotbugs/spotbugs/issues/2024)) ### Fixed - Bumped Saxon-HE from 10.6 to 11.3 ([#1955](https://github.com/spotbugs/spotbugs/pull/1955), [#1999](https://github.com/spotbugs/spotbugs/pull/1999)) diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/Plugin.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/Plugin.java index 11bc795b290..7322bdab04b 100644 --- a/spotbugs/src/main/java/edu/umd/cs/findbugs/Plugin.java +++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/Plugin.java @@ -20,10 +20,12 @@ package edu.umd.cs.findbugs; import java.io.File; +import java.io.IOException; import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; +import java.net.URLClassLoader; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -53,7 +55,7 @@ * @see PluginLoader * @author David Hovemeyer */ -public class Plugin { +public class Plugin implements AutoCloseable { private static final String USE_FINDBUGS_VERSION = "USE_FINDBUGS_VERSION"; @@ -696,4 +698,12 @@ public static synchronized void removeCustomPlugin(Plugin plugin) { DetectorFactoryCollection.instance().unLoadPlugin(plugin); } + /** + * Closes the underlying {@link PluginLoader}, in turn this closes the {@link URLClassLoader}. + * When loading a custom plugin from a .jar file this method needs to be called to release the reference on that file. + */ + @Override + public void close() throws IOException { + pluginLoader.close(); + } } diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/PluginLoader.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/PluginLoader.java index c51e9dea785..c7ccafbe2d5 100644 --- a/spotbugs/src/main/java/edu/umd/cs/findbugs/PluginLoader.java +++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/PluginLoader.java @@ -102,7 +102,7 @@ * @see Plugin * @see PluginException */ -public class PluginLoader { +public class PluginLoader implements AutoCloseable { private static final String XPATH_PLUGIN_SHORT_DESCRIPTION = "/MessageCollection/Plugin/ShortDescription"; private static final String XPATH_PLUGIN_WEBSITE = "/FindbugsPlugin/@website"; @@ -140,6 +140,9 @@ public class PluginLoader { private final URI loadedFromUri; + // The classloaders we have created, so we can close then if we want to close the plugin + private List createdClassLoaders = new ArrayList<>(); + /** plugin Id for parent plugin */ String parentId; @@ -196,7 +199,7 @@ public boolean hasParent() { */ private PluginLoader(@Nonnull URL url, URI uri, ClassLoader parent, boolean isInitial, boolean optional) throws PluginException { URL[] loaderURLs = createClassloaderUrls(url); - classLoaderForResources = new URLClassLoader(loaderURLs); + classLoaderForResources = buildURLClassLoader(loaderURLs); loadedFrom = url; loadedFromUri = uri; jarName = getJarName(url); @@ -205,7 +208,7 @@ private PluginLoader(@Nonnull URL url, URI uri, ClassLoader parent, boolean isIn optionalPlugin = optional; plugin = init(); if (!hasParent()) { - classLoader = new URLClassLoader(loaderURLs, parent); + classLoader = buildURLClassLoader(loaderURLs, parent); } else { if (parent != PluginLoader.class.getClassLoader()) { throw new IllegalArgumentException("Can't specify parentid " + parentId + " and provide a separate class loader"); @@ -213,7 +216,7 @@ private PluginLoader(@Nonnull URL url, URI uri, ClassLoader parent, boolean isIn Plugin parentPlugin = Plugin.getByPluginId(parentId); if (parentPlugin != null) { parent = parentPlugin.getClassLoader(); - classLoader = new URLClassLoader(loaderURLs, parent); + classLoader = buildURLClassLoader(loaderURLs, parent); } } if (classLoader == null) { @@ -246,7 +249,7 @@ private static void finishLazyInitialization() { i.remove(); try { URL[] loaderURLs = PluginLoader.createClassloaderUrls(pluginLoader.loadedFrom); - pluginLoader.classLoader = new URLClassLoader(loaderURLs, parent.getClassLoader()); + pluginLoader.classLoader = pluginLoader.buildURLClassLoader(loaderURLs, parent.getClassLoader()); pluginLoader.loadPluginComponents(); Plugin.putPlugin(pluginLoader.loadedFromUri, pluginLoader.plugin); } catch (PluginException e) { @@ -275,6 +278,50 @@ private static void finishLazyInitialization() { lazyInitialization = false; } + /** + * Creates a new {@link URLClassLoader} and adds it to the list of classloaders + * we need to close if we close the corresponding plugin + * + * @param + * urls the URLs from which to load classes and resources + * @return a new {@link URLClassLoader} + */ + private URLClassLoader buildURLClassLoader(URL[] urls) { + URLClassLoader urlClassLoader = new URLClassLoader(urls); + createdClassLoaders.add(urlClassLoader); + + return urlClassLoader; + } + + /** + * Creates a new {@link URLClassLoader} and adds it to the list of classloaders + * we need to close if we close the corresponding plugin + * + * @param + * urls the URLs from which to load classes and resources + * @param + * parent the parent class loader for delegation + * @return a new {@link URLClassLoader} + */ + private URLClassLoader buildURLClassLoader(URL[] urls, ClassLoader parent) { + URLClassLoader urlClassLoader = new URLClassLoader(urls, parent); + createdClassLoaders.add(urlClassLoader); + + return urlClassLoader; + } + + /** + * Closes the class loaders created in this {@link PluginLoader} + * @throws IOException if a class loader fails to close, in that case the other classloaders won't be closed + */ + @Override + public void close() throws IOException { + for (URLClassLoader urlClassLoader : createdClassLoaders) { + urlClassLoader.close(); + } + } + + /** * Patch for issue 3429143: allow plugins load classes/resources from 3rd * party jars