From 69e017b9ea1d488c8d73dc1d2cbb55bab33dc917 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 a88733f1c..6ab9cd821 100644 --- a/src/main/java/org/bukkit/plugin/PluginLoader.java +++ b/src/main/java/org/bukkit/plugin/PluginLoader.java @@ -77,4 +77,18 @@ public interface PluginLoader { * @param plugin Plugin to disable */ public void disablePlugin(@NotNull 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(@NotNull 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 41e26451f..86cc5025a 100644 --- a/src/main/java/org/bukkit/plugin/PluginManager.java +++ b/src/main/java/org/bukkit/plugin/PluginManager.java @@ -161,6 +161,18 @@ public interface PluginManager { */ public void disablePlugin(@NotNull 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(@NotNull 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 8ddaf9c2e..8bb24f734 100644 --- a/src/main/java/org/bukkit/plugin/SimplePluginManager.java +++ b/src/main/java/org/bukkit/plugin/SimplePluginManager.java @@ -492,17 +492,28 @@ public final class SimplePluginManager implements PluginManager { @Override 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 } } @Override public void disablePlugin(@NotNull final Plugin plugin) { + disablePlugin(plugin, false); + } + + @Override + public void disablePlugin(@NotNull 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 @@ -557,7 +568,7 @@ public final class SimplePluginManager implements PluginManager { @Override public void clearPlugins() { synchronized (this) { - disablePlugins(); + disablePlugins(true); // Paper - close Classloader on disable plugins.clear(); lookupNames.clear(); dependencyGraph = GraphBuilder.directed().build(); diff --git a/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java b/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java index e72cbde4b..32ec68b48 100644 --- a/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java +++ b/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java @@ -331,7 +331,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 } @@ -344,6 +344,12 @@ public final class JavaPluginLoader implements PluginLoader { @Override public void disablePlugin(@NotNull Plugin plugin) { + // Paper start - close Classloader on disable + disablePlugin(plugin, false); // Retain old behavior unless requested + } + + public void disablePlugin(@NotNull 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()) { @@ -370,6 +376,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.25.1