OpenSource/PaperMC
OpenSource/PaperMC
1
0
Fork 0
mirror of https://github.com/PaperMC/Paper.git synced 2024-11-23 23:57:43 +01:00
Code Issues Projects Releases Packages Wiki Activity

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

Browse source
This commit is contained in:
Jason 2021-10-06 23:00:32 -05:00 committed by GitHub
parent a4199412fc
commit 8e661c6b6e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 59 additions and 95 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
patches/api/0016-Automatically-disable-plugins-that-fail-to-load.patch
View file

@ -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
}

130
patches/api/0101-Close-Plugin-Class-Loaders-on-Disable.patch
View file

@ -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 {
}
}
+ // 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) {
+ 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 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
}
}
}

16
patches/api/0132-Remove-deadlock-risk-in-firing-async-events.patch
View file

@ -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) {

4
patches/api/0195-Disable-Sync-Events-firing-Async-errors-during-shutd.patch
View file

@ -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.");