From 0000000000000000000000000000000000000000 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 9f32b57464352c08617f6adec144111b8fcad50c..5af99aa87e6d4fbff81bd9de40484f4ed5a8a3ba 100644
--- a/src/main/java/org/bukkit/plugin/SimplePluginManager.java
+++ b/src/main/java/org/bukkit/plugin/SimplePluginManager.java
@@ -468,7 +468,7 @@ public final class SimplePluginManager implements PluginManager {
      * @return true if the plugin is enabled, otherwise false
      */
     @Override
-    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 {
@@ -477,7 +477,7 @@ public final class SimplePluginManager implements PluginManager {
     }
 
     @Override
-    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);
 
@@ -520,7 +520,7 @@ public final class SimplePluginManager implements PluginManager {
     // Paper end
 
     @Override
-    public void disablePlugin(@NotNull final Plugin plugin) {
+    public synchronized void disablePlugin(@NotNull final Plugin plugin) { // Paper - synchronize
         if (plugin.isEnabled()) {
             try {
                 plugin.getPluginLoader().disablePlugin(plugin);
@@ -589,6 +589,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.
@@ -597,23 +598,13 @@ public final class SimplePluginManager implements PluginManager {
      */
     @Override
     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.");
-            }
-        } else {
-            if (!server.isPrimaryThread()) {
-                throw new IllegalStateException(event.getEventName() + " cannot be triggered asynchronously from another thread.");
-            }
+        // Paper - replace callEvent by merging to below method
+        if (event.isAsynchronous() && server.isPrimaryThread()) {
+            throw new IllegalStateException(event.getEventName() + " may only be triggered asynchronously.");
+        } else if (!event.isAsynchronous() && !server.isPrimaryThread()) {
+            throw new IllegalStateException(event.getEventName() + " may only be triggered synchronously.");
         }
 
-        fireEvent(event);
-    }
-
-    private void fireEvent(@NotNull Event event) {
         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 c46ed2acb82db814d660459b705dd49e6d44240f..24dc87898e0fc40dfaf52f17a1bd26eccfbc7abc 100644
--- a/src/test/java/org/bukkit/plugin/PluginManagerTest.java
+++ b/src/test/java/org/bukkit/plugin/PluginManagerTest.java
@@ -16,7 +16,7 @@ public class PluginManagerTest {
     private static final PluginManager pm = org.bukkit.Bukkit.getServer().getPluginManager(); // Paper
 
     private final MutableObject store = new MutableObject();
-
+/* // Paper start - remove unneeded test
     @Test
     public void testAsyncSameThread() {
         final Event event = new TestEvent(true);
@@ -27,14 +27,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);
@@ -128,7 +128,7 @@ public class PluginManagerTest {
         if (store.value == null) {
             throw new IllegalStateException("No exception thrown");
         }
-    }
+    } */ // Paper
 
     @Test
     public void testRemovePermissionByNameLower() {