PaperMC/Spigot-API-Patches/0130-Remove-deadlock-risk-in-firing-async-events.patch
Shane Freeder ea855e2b46 Updated Upstream (Bukkit/CraftBukkit/Spigot)
Upstream has released updates that appears to apply and compile correctly.
This update has not been tested by PaperMC and as with ANY update, please do your own testing

Developers!: You will need to clean up your work/Minecraft/1.13.2 folder
for this

Also, restore a patch that was dropped in the last upstream

Bukkit Changes:
279eeab3 Fix command description not being set
96e2bb18 Remove debug print from SyntheticEventTest

CraftBukkit Changes:
d3ed1516 Fix dangerously threaded beacons
217a293d Don't relocate joptsimple to allow --help to work.
1be05a21 Prepare for imminent Java 12 release
a49270b2 Mappings Update
5259d80c SPIGOT-4669: Fix PlayerTeleportEvent coordinates for relative teleports

Spigot Changes:
e6eb36f2 Rebuild patches
2019-03-20 01:55:16 +00:00

122 lines
5 KiB
Diff

From 0c86016a1fb9b9fe7f97b02ce776db7f308c5381 Mon Sep 17 00:00:00 2001
From: Aikar <aikar@aikar.co>
Date: Sun, 9 Sep 2018 00:32:05 -0400
Subject: [PATCH] Remove deadlock risk in firing async events
The PluginManager incorrectly used synchronization on firing any event
that was marked as synchronous.
This synchronized did not even protect any concurrency risk as
handlers were already thread safe in terms of mutations during event
dispatch.
The way it was used, has commonly led to deadlocks on the server,
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 7d4ca43b5..9ec042fd7 100644
--- a/src/main/java/org/bukkit/plugin/SimplePluginManager.java
+++ b/src/main/java/org/bukkit/plugin/SimplePluginManager.java
@@ -392,7 +392,7 @@ public final class SimplePluginManager implements PluginManager {
* @param plugin Plugin to check
* @return true if the plugin is enabled, otherwise false
*/
- public boolean isPluginEnabled(@Nullable Plugin plugin) {
+ public synchronized boolean isPluginEnabled(@Nullable Plugin plugin) { // Paper - synchronize
if ((plugin != null) && (plugins.contains(plugin))) {
return plugin.isEnabled();
} else {
@@ -400,7 +400,7 @@ public final class SimplePluginManager implements PluginManager {
}
}
- public void enablePlugin(@NotNull final Plugin plugin) {
+ public synchronized void enablePlugin(@NotNull final Plugin plugin) { // Paper - synchronize
if (!plugin.isEnabled()) {
List<Command> pluginCommands = PluginCommandYamlParser.parse(plugin);
@@ -437,7 +437,7 @@ public final class SimplePluginManager implements PluginManager {
disablePlugin(plugin, false);
}
- 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
if (plugin.isEnabled()) {
try {
@@ -497,6 +497,7 @@ public final class SimplePluginManager implements PluginManager {
defaultPerms.get(false).clear();
}
}
+ private void fireEvent(Event event) { callEvent(event); } // Paper - support old method incase plugin uses reflection
/**
* Calls an event with the given details.
@@ -506,22 +507,7 @@ public final class SimplePluginManager implements PluginManager {
* @param event Event details
*/
public void callEvent(@NotNull Event event) {
- if (event.isAsynchronous()) {
- if (Thread.holdsLock(this)) {
- throw new IllegalStateException(event.getEventName() + " cannot be triggered asynchronously from inside synchronized code.");
- }
- if (server.isPrimaryThread()) {
- throw new IllegalStateException(event.getEventName() + " cannot be triggered asynchronously from primary server thread.");
- }
- fireEvent(event);
- } else {
- synchronized (this) {
- fireEvent(event);
- }
- }
- }
-
- private void fireEvent(@NotNull Event event) {
+ // Paper - replace callEvent by merging to below method
HandlerList handlers = event.getHandlers();
RegisteredListener[] listeners = handlers.getRegisteredListeners();
diff --git a/src/test/java/org/bukkit/plugin/PluginManagerTest.java b/src/test/java/org/bukkit/plugin/PluginManagerTest.java
index 6b86128e1..56308c0c6 100644
--- a/src/test/java/org/bukkit/plugin/PluginManagerTest.java
+++ b/src/test/java/org/bukkit/plugin/PluginManagerTest.java
@@ -19,7 +19,7 @@ public class PluginManagerTest {
private static final PluginManager pm = TestServer.getInstance().getPluginManager();
private final MutableObject store = new MutableObject();
-
+/* // Paper start - remove unneeded test
@Test
public void testAsyncSameThread() {
final Event event = new TestEvent(true);
@@ -30,14 +30,14 @@ public class PluginManagerTest {
return;
}
throw new IllegalStateException("No exception thrown");
- }
+ }*/ // Paper end
@Test
public void testSyncSameThread() {
final Event event = new TestEvent(false);
pm.callEvent(event);
}
-
+/* // Paper start - remove unneeded test
@Test
public void testAsyncLocked() throws InterruptedException {
final Event event = new TestEvent(true);
@@ -58,7 +58,7 @@ public class PluginManagerTest {
secondThread.join();
assertThat(store.value, is(instanceOf(IllegalStateException.class)));
assertThat(event.getEventName() + " cannot be triggered asynchronously from inside synchronized code.", is(((Throwable) store.value).getMessage()));
- }
+ }*/ // Paper end
@Test
public void testAsyncUnlocked() throws InterruptedException {
--
2.21.0