From c5d36c89c26360428075ecd830da4f65b2c2de11 Mon Sep 17 00:00:00 2001 From: Aikar Date: Tue, 1 May 2018 21:33:35 -0400 Subject: [PATCH] Close Plugin Class Loaders on Disable This should close more memory leaks from /reload and disabling plugins, by closing the class loader and the jar file. diff --git a/src/main/java/org/bukkit/plugin/PluginLoader.java b/src/main/java/org/bukkit/plugin/PluginLoader.java index e7981a1d..d34756f1 100644 --- a/src/main/java/org/bukkit/plugin/PluginLoader.java +++ b/src/main/java/org/bukkit/plugin/PluginLoader.java @@ -73,4 +73,19 @@ public interface PluginLoader { * @param plugin Plugin to disable */ public void disablePlugin(Plugin plugin); + + // Paper start - close Classloader on disable + /** + * Disables the specified plugin + *

+ * Attempting to disable a plugin that is not enabled will have no effect + * + * @param plugin Plugin to disable + * @param closeClassloader if the classloader for the Plugin should be closed + */ + // provide default to allow other PluginLoader implementations to work + default public void disablePlugin(Plugin plugin, boolean closeClassloader) { + disablePlugin(plugin); + } + // Paper end - close Classloader on disable } diff --git a/src/main/java/org/bukkit/plugin/PluginManager.java b/src/main/java/org/bukkit/plugin/PluginManager.java index e5638d56..b72d5a9b 100644 --- a/src/main/java/org/bukkit/plugin/PluginManager.java +++ b/src/main/java/org/bukkit/plugin/PluginManager.java @@ -154,6 +154,18 @@ public interface PluginManager { */ public void disablePlugin(Plugin plugin); + // Paper start - close Classloader on disable + /** + * Disables the specified plugin + *

+ * Attempting to disable a plugin that is not enabled will have no effect + * + * @param plugin Plugin to disable + * @param closeClassloader if the classloader for the Plugin should be closed + */ + public void disablePlugin(Plugin plugin, boolean closeClassloader); + // Paper end - close Classloader on disable + /** * Gets a {@link Permission} from its fully qualified name * diff --git a/src/main/java/org/bukkit/plugin/SimplePluginManager.java b/src/main/java/org/bukkit/plugin/SimplePluginManager.java index bd0588a2..cb2b0b9c 100644 --- a/src/main/java/org/bukkit/plugin/SimplePluginManager.java +++ b/src/main/java/org/bukkit/plugin/SimplePluginManager.java @@ -412,17 +412,29 @@ public final class SimplePluginManager implements PluginManager { } } + // Paper start - close Classloader on disable public void disablePlugins() { + disablePlugins(false); + } + + public void disablePlugins(boolean closeClassloaders) { + // Paper end - close Classloader on disable Plugin[] plugins = getPlugins(); for (int i = plugins.length - 1; i >= 0; i--) { - disablePlugin(plugins[i]); + disablePlugin(plugins[i], closeClassloaders); // Paper - close Classloader on disable } } - public void disablePlugin(final Plugin plugin) { + // Paper start - close Classloader on disable + public void disablePlugin(Plugin plugin) { + disablePlugin(plugin, false); + } + + public void disablePlugin(final Plugin plugin, boolean closeClassloader) { + // Paper end - close Classloader on disable if (plugin.isEnabled()) { try { - plugin.getPluginLoader().disablePlugin(plugin); + plugin.getPluginLoader().disablePlugin(plugin, closeClassloader); // Paper - close Classloader on disable } catch (Throwable ex) { handlePluginException("Error occurred (in the plugin loader) while disabling " + plugin.getDescription().getFullName() + " (Is it up to date?)", ex, plugin); // Paper @@ -468,7 +480,7 @@ public final class SimplePluginManager implements PluginManager { public void clearPlugins() { synchronized (this) { - disablePlugins(); + disablePlugins(true); // Paper - close Classloader on disable plugins.clear(); lookupNames.clear(); HandlerList.unregisterAll(); diff --git a/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java b/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java index 77207f14..c7782e7d 100644 --- a/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java +++ b/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java @@ -320,7 +320,7 @@ public final class JavaPluginLoader implements PluginLoader { } catch (Throwable ex) { server.getLogger().log(Level.SEVERE, "Error occurred while enabling " + plugin.getDescription().getFullName() + " (Is it up to date?)", ex); // Paper start - Disable plugins that fail to load - disablePlugin(jPlugin); + server.getPluginManager().disablePlugin(jPlugin, true); // Paper - close Classloader on disable - She's dead jim return; // Paper end } @@ -331,7 +331,13 @@ public final class JavaPluginLoader implements PluginLoader { } } + // Paper start - close Classloader on disable public void disablePlugin(Plugin plugin) { + disablePlugin(plugin, false); // Retain old behavior unless requested + } + + public void disablePlugin(Plugin plugin, boolean closeClassloader) { + // Paper end - close Class Loader on disable Validate.isTrue(plugin instanceof JavaPlugin, "Plugin is not associated with this PluginLoader"); if (plugin.isEnabled()) { @@ -358,6 +364,16 @@ public final class JavaPluginLoader implements PluginLoader { for (String name : names) { removeClass(name); } + // Paper start - close Class Loader on disable + try { + if (closeClassloader) { + loader.close(); + } + } catch (IOException e) { + server.getLogger().log(Level.WARNING, "Error closing the Plugin Class Loader for " + plugin.getDescription().getFullName()); + e.printStackTrace(); + } + // Paper end } } } -- 2.20.1