Deprecate API methods added by 'Close Plugin Class Loaders on Disable' (#6737)

4 files changed, 59 insertions, 95 deletions

diff --git a/patches/api/0016-Automatically-disable-plugins-that-fail-to-load.patch b/patches/api/0016-Automatically-disable-plugins-that-fail-to-load.patch
index 55aeb559ba..fcf406e1d2 100644
--- a/patches/api/0016-Automatically-disable-plugins-that-fail-to-load.patch
+++ b/patches/api/0016-Automatically-disable-plugins-that-fail-to-load.patch
@@ -5,7 +5,7 @@ Subject: [PATCH] Automatically disable plugins that fail to load
 
 
 diff --git a/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java b/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java
-index cf2f517765d8f2a23cc4a17d9ee2dcd81f841b1b..2e306c7b984a02e12a74fac14589bf29ab6488bf 100644
+index cf2f517765d8f2a23cc4a17d9ee2dcd81f841b1b..a3bc4155536f612ee2ae38ec7f16b974bdd24ab2 100644
 --- a/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java
 +++ b/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java
 @@ -335,6 +335,10 @@ public final class JavaPluginLoader implements PluginLoader {
@@ -13,7 +13,7 @@ index cf2f517765d8f2a23cc4a17d9ee2dcd81f841b1b..2e306c7b984a02e12a74fac14589bf29
              } 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);
++                this.server.getPluginManager().disablePlugin(jPlugin);
 +                return;
 +                // Paper end
              }
diff --git a/patches/api/0101-Close-Plugin-Class-Loaders-on-Disable.patch b/patches/api/0101-Close-Plugin-Class-Loaders-on-Disable.patch
index e6b629f072..ae4dc00ba4 100644
--- a/patches/api/0101-Close-Plugin-Class-Loaders-on-Disable.patch
+++ b/patches/api/0101-Close-Plugin-Class-Loaders-on-Disable.patch
@@ -6,23 +6,32 @@ 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.
 
+Note: This patch is no longer necessary as upstream now also closes
+PluginClassLoaders on disable. This patch is now only to keep around
+API methods it previously added, and to add back the log message when a
+PluginClassLoader fails to disable, as upstream decided to ignore the
+exception.
+
 diff --git a/src/main/java/org/bukkit/plugin/PluginLoader.java b/src/main/java/org/bukkit/plugin/PluginLoader.java
-index a88733f1cd1ddb5d85ab1b0e6af4fd5b80bbc1c6..6ab9cd8213cbe35943748dcf42948d5fc048c84c 100644
+index a88733f1cd1ddb5d85ab1b0e6af4fd5b80bbc1c6..256e440e699942e3c9da4205bb964bdc10ec92c4 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 {
+@@ -77,4 +77,21 @@ public interface PluginLoader {
       * @param plugin Plugin to disable
       */
      public void disablePlugin(@NotNull Plugin plugin);
++
 +    // Paper start - close Classloader on disable
 +    /**
-+     * Disables the specified plugin
-+     * <p>
-+     * Attempting to disable a plugin that is not enabled will have no effect
++     * This method is no longer useful as upstream has
++     * made it so plugin classloaders are always closed on disable.
++     * Use {@link #disablePlugin(Plugin)} instead.
 +     *
 +     * @param plugin Plugin to disable
-+     * @param closeClassloader if the classloader for the Plugin should be closed
++     * @param closeClassloader unused
++     * @deprecated Classloader is always closed by upstream now.
 +     */
++    @Deprecated(forRemoval = true)
 +    // provide default to allow other PluginLoader implementations to work
 +    default public void disablePlugin(@NotNull Plugin plugin, boolean closeClassloader) {
 +        disablePlugin(plugin);
@@ -30,112 +39,67 @@ index a88733f1cd1ddb5d85ab1b0e6af4fd5b80bbc1c6..6ab9cd8213cbe35943748dcf42948d5f
 +    // 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 41e26451fe12d8e6e0ef73c85731b24b4e3f200c..86cc5025ad98f7a752c51713b7cd6a39d5136ecc 100644
+index 41e26451fe12d8e6e0ef73c85731b24b4e3f200c..0d1b20f2b5580ea5505ccc2f003925dbcee67199 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 {
+@@ -161,6 +161,22 @@ public interface PluginManager {
       */
      public void disablePlugin(@NotNull Plugin plugin);
  
 +    // Paper start - close Classloader on disable
 +    /**
-+     * Disables the specified plugin
-+     * <p>
-+     * Attempting to disable a plugin that is not enabled will have no effect
++     * This method is no longer useful as upstream has
++     * made it so plugin classloaders are always closed on disable.
++     * Use {@link #disablePlugin(Plugin)} instead.
 +     *
 +     * @param plugin Plugin to disable
-+     * @param closeClassloader if the classloader for the Plugin should be closed
++     * @param closeClassloader unused
++     * @deprecated Classloader is always closed by upstream now.
 +     */
-+    public void disablePlugin(@NotNull Plugin plugin, boolean closeClassloader);
++    @Deprecated(forRemoval = true)
++    public default void disablePlugin(@NotNull Plugin plugin, boolean closeClassloader) {
++        this.disablePlugin(plugin);
++    }
 +    // 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 2d27dfb859c312d46a14d0356c7c3f227e965a67..892ec4b43cc97a235df0819d30391a8a3770cbcb 100644
+index 2d27dfb859c312d46a14d0356c7c3f227e965a67..7867243a8ed67416895cdcd949ac424f5d29d98b 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
+@@ -498,6 +498,21 @@ public final class SimplePluginManager implements PluginManager {
          }
      }
  
-     @Override
-     public void disablePlugin(@NotNull final Plugin plugin) {
-+        disablePlugin(plugin, false);
++    // Paper start
++    /**
++     * This method is no longer useful as upstream has
++     * made it so plugin classloaders are always closed on disable.
++     * Use {@link #disablePlugins()} instead.
++     *
++     * @param closeClassloaders unused
++     * @deprecated Classloader is always closed by upstream now.
++     */
++    @Deprecated(forRemoval = true)
++    public void disablePlugins(boolean closeClassloaders) {
++        this.disablePlugins();
 +    }
++    // Paper end
 +
-+    @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();
+     public void disablePlugin(@NotNull final Plugin plugin) {
+         if (plugin.isEnabled()) {
 diff --git a/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java b/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java
-index 79ac529017aac059d13fe342f279e9c8faeba599..816c2b1797447ab315ceb6eda89d25f27d2bce98 100644
+index b7cd98f20de3421cd3b8a95cfa2155fa77afa8b1..7483ceff298cafa244bd99316a0338aa075267d4 100644
 --- a/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java
 +++ b/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java
-@@ -322,7 +322,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
-             }
-@@ -335,6 +335,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()) {
-@@ -367,6 +373,16 @@ public final class JavaPluginLoader implements PluginLoader {
+@@ -366,6 +366,7 @@ public final class JavaPluginLoader implements PluginLoader {
+                     loader.close();
                  } catch (IOException ex) {
                      //
++                    this.server.getLogger().log(Level.WARNING, "Error closing the PluginClassLoader for '" + plugin.getDescription().getFullName() + "'", ex); // Paper - log exception
                  }
-+                // 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
              }
          }
-     }
diff --git a/patches/api/0132-Remove-deadlock-risk-in-firing-async-events.patch b/patches/api/0132-Remove-deadlock-risk-in-firing-async-events.patch
index 27e5ca7dfe..3f5f5d932c 100644
--- a/patches/api/0132-Remove-deadlock-risk-in-firing-async-events.patch
+++ b/patches/api/0132-Remove-deadlock-risk-in-firing-async-events.patch
@@ -16,7 +16,7 @@ which results in a hard crash.
 This change removes the synchronize and adds some protection around enable/disable
 
 diff --git a/src/main/java/org/bukkit/plugin/SimplePluginManager.java b/src/main/java/org/bukkit/plugin/SimplePluginManager.java
-index 892ec4b43cc97a235df0819d30391a8a3770cbcb..b83637f872be5fc73500b10c917d71802976b340 100644
+index 7867243a8ed67416895cdcd949ac424f5d29d98b..a3027c1c6109bcb20d0468f6d0cd37182bb279ea 100644
 --- a/src/main/java/org/bukkit/plugin/SimplePluginManager.java
 +++ b/src/main/java/org/bukkit/plugin/SimplePluginManager.java
 @@ -462,7 +462,7 @@ public final class SimplePluginManager implements PluginManager {
@@ -37,16 +37,16 @@ index 892ec4b43cc97a235df0819d30391a8a3770cbcb..b83637f872be5fc73500b10c917d7180
          if (!plugin.isEnabled()) {
              List<Command> pluginCommands = PluginCommandYamlParser.parse(plugin);
  
-@@ -509,7 +509,7 @@ public final class SimplePluginManager implements PluginManager {
-     }
+@@ -514,7 +514,7 @@ public final class SimplePluginManager implements PluginManager {
+     // Paper end
  
      @Override
--    public void disablePlugin(@NotNull final Plugin plugin, boolean closeClassloader) {
-+    public synchronized void disablePlugin(@NotNull final Plugin plugin, boolean closeClassloader) { // Paper - synchronize
-         // Paper end - close Classloader on disable
+-    public void disablePlugin(@NotNull final Plugin plugin) {
++    public synchronized void disablePlugin(@NotNull final Plugin plugin) { // Paper - synchronize
          if (plugin.isEnabled()) {
              try {
-@@ -579,6 +579,7 @@ public final class SimplePluginManager implements PluginManager {
+                 plugin.getPluginLoader().disablePlugin(plugin);
+@@ -583,6 +583,7 @@ public final class SimplePluginManager implements PluginManager {
              defaultPerms.get(false).clear();
          }
      }
@@ -54,7 +54,7 @@ index 892ec4b43cc97a235df0819d30391a8a3770cbcb..b83637f872be5fc73500b10c917d7180
  
      /**
       * Calls an event with the given details.
-@@ -587,23 +588,13 @@ public final class SimplePluginManager implements PluginManager {
+@@ -591,23 +592,13 @@ public final class SimplePluginManager implements PluginManager {
       */
      @Override
      public void callEvent(@NotNull Event event) {
diff --git a/patches/api/0195-Disable-Sync-Events-firing-Async-errors-during-shutd.patch b/patches/api/0195-Disable-Sync-Events-firing-Async-errors-during-shutd.patch
index 32392efc4e..6018c18f9b 100644
--- a/patches/api/0195-Disable-Sync-Events-firing-Async-errors-during-shutd.patch
+++ b/patches/api/0195-Disable-Sync-Events-firing-Async-errors-during-shutd.patch
@@ -11,10 +11,10 @@ errors.
 This isn't an issue on Spigot
 
 diff --git a/src/main/java/org/bukkit/plugin/SimplePluginManager.java b/src/main/java/org/bukkit/plugin/SimplePluginManager.java
-index b83637f872be5fc73500b10c917d71802976b340..49e5d49eb09bb966e47d6a03ac08a527c963b43d 100644
+index a3027c1c6109bcb20d0468f6d0cd37182bb279ea..20b4ef7a94e00d9264b8ecc126ce5853b584ea8c 100644
 --- a/src/main/java/org/bukkit/plugin/SimplePluginManager.java
 +++ b/src/main/java/org/bukkit/plugin/SimplePluginManager.java
-@@ -591,7 +591,7 @@ public final class SimplePluginManager implements PluginManager {
+@@ -595,7 +595,7 @@ public final class SimplePluginManager implements PluginManager {
          // Paper - replace callEvent by merging to below method
          if (event.isAsynchronous() && server.isPrimaryThread()) {
              throw new IllegalStateException(event.getEventName() + " may only be triggered asynchronously.");