2019-05-27 09:13:41 +00:00
|
|
|
From 2817ea0cd2b735fcf41d322476b4b1d6c97f39a4 Mon Sep 17 00:00:00 2001
|
2018-09-09 05:04:29 +00:00
|
|
|
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
|
|
|
|
|
2019-04-23 04:47:07 +00:00
|
|
|
diff --git a/src/main/java/org/bukkit/entity/Entity.java b/src/main/java/org/bukkit/entity/Entity.java
|
2019-05-06 02:58:04 +00:00
|
|
|
index adca48f1..2d3cee14 100644
|
2019-04-23 04:47:07 +00:00
|
|
|
--- a/src/main/java/org/bukkit/entity/Entity.java
|
|
|
|
+++ b/src/main/java/org/bukkit/entity/Entity.java
|
2019-05-06 02:58:04 +00:00
|
|
|
@@ -28,7 +28,7 @@ import org.jetbrains.annotations.Nullable;
|
2019-04-23 04:47:07 +00:00
|
|
|
*/
|
2019-05-06 02:58:04 +00:00
|
|
|
public interface Entity extends Metadatable, CommandSender, Nameable, PersistentDataHolder {
|
2019-04-23 04:47:07 +00:00
|
|
|
|
|
|
|
- /**
|
|
|
|
+ /*
|
|
|
|
* Gets the entity's current position
|
|
|
|
*
|
|
|
|
* @return a new copy of Location containing the position of this entity
|
2018-09-09 05:04:29 +00:00
|
|
|
diff --git a/src/main/java/org/bukkit/plugin/SimplePluginManager.java b/src/main/java/org/bukkit/plugin/SimplePluginManager.java
|
2019-05-06 02:58:04 +00:00
|
|
|
index da3cd63b..583f8ee4 100644
|
2018-09-09 05:04:29 +00:00
|
|
|
--- a/src/main/java/org/bukkit/plugin/SimplePluginManager.java
|
|
|
|
+++ b/src/main/java/org/bukkit/plugin/SimplePluginManager.java
|
2019-05-06 02:58:04 +00:00
|
|
|
@@ -398,7 +398,7 @@ public final class SimplePluginManager implements PluginManager {
|
2018-09-09 05:04:29 +00:00
|
|
|
* @return true if the plugin is enabled, otherwise false
|
|
|
|
*/
|
2019-05-06 02:58:04 +00:00
|
|
|
@Override
|
2019-03-20 00:28:15 +00:00
|
|
|
- public boolean isPluginEnabled(@Nullable Plugin plugin) {
|
|
|
|
+ public synchronized boolean isPluginEnabled(@Nullable Plugin plugin) { // Paper - synchronize
|
2018-09-09 05:04:29 +00:00
|
|
|
if ((plugin != null) && (plugins.contains(plugin))) {
|
|
|
|
return plugin.isEnabled();
|
|
|
|
} else {
|
2019-05-06 02:58:04 +00:00
|
|
|
@@ -407,7 +407,7 @@ public final class SimplePluginManager implements PluginManager {
|
2018-09-09 05:04:29 +00:00
|
|
|
}
|
|
|
|
|
2019-05-06 02:58:04 +00:00
|
|
|
@Override
|
2019-03-20 00:28:15 +00:00
|
|
|
- public void enablePlugin(@NotNull final Plugin plugin) {
|
|
|
|
+ public synchronized void enablePlugin(@NotNull final Plugin plugin) { // Paper - synchronize
|
2018-09-09 05:04:29 +00:00
|
|
|
if (!plugin.isEnabled()) {
|
|
|
|
List<Command> pluginCommands = PluginCommandYamlParser.parse(plugin);
|
|
|
|
|
2019-05-06 02:58:04 +00:00
|
|
|
@@ -445,7 +445,7 @@ public final class SimplePluginManager implements PluginManager {
|
2018-09-09 05:04:29 +00:00
|
|
|
}
|
|
|
|
|
2019-05-06 02:58:04 +00:00
|
|
|
@Override
|
2019-03-20 00:28:15 +00:00
|
|
|
- public void disablePlugin(@NotNull final Plugin plugin, boolean closeClassloader) {
|
|
|
|
+ public synchronized void disablePlugin(@NotNull final Plugin plugin, boolean closeClassloader) { // Paper - synchronize
|
2018-09-09 05:04:29 +00:00
|
|
|
// Paper end - close Classloader on disable
|
|
|
|
if (plugin.isEnabled()) {
|
|
|
|
try {
|
2019-05-06 02:58:04 +00:00
|
|
|
@@ -506,6 +506,7 @@ public final class SimplePluginManager implements PluginManager {
|
2018-09-09 05:04:29 +00:00
|
|
|
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.
|
2019-05-06 02:58:04 +00:00
|
|
|
@@ -516,25 +517,11 @@ public final class SimplePluginManager implements PluginManager {
|
2018-09-09 05:04:29 +00:00
|
|
|
*/
|
2019-05-06 02:58:04 +00:00
|
|
|
@Override
|
2019-03-20 00:28:15 +00:00
|
|
|
public void callEvent(@NotNull Event event) {
|
2018-09-09 05:04:29 +00:00
|
|
|
- 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 {
|
2019-04-23 04:47:07 +00:00
|
|
|
- if (!server.isPrimaryThread()) {
|
|
|
|
- throw new IllegalStateException(event.getEventName() + " cannot be triggered asynchronously from another thread.");
|
|
|
|
- }
|
2018-09-09 05:04:29 +00:00
|
|
|
- synchronized (this) {
|
|
|
|
- fireEvent(event);
|
|
|
|
- }
|
2019-04-23 04:47:07 +00:00
|
|
|
+ // Paper - replace callEvent by merging to below method
|
|
|
|
+ if (event.isAsynchronous() && server.isPrimaryThread()) {
|
|
|
|
+ throw new IllegalStateException(event.getEventName() + " cannot be triggered asynchronously from primary server thread.");
|
|
|
|
}
|
2018-09-09 05:04:29 +00:00
|
|
|
- }
|
2019-04-23 04:47:07 +00:00
|
|
|
|
2019-03-20 00:28:15 +00:00
|
|
|
- private void fireEvent(@NotNull Event event) {
|
2018-09-09 05:04:29 +00:00
|
|
|
HandlerList handlers = event.getHandlers();
|
|
|
|
RegisteredListener[] listeners = handlers.getRegisteredListeners();
|
|
|
|
|
2018-09-09 05:14:18 +00:00
|
|
|
diff --git a/src/test/java/org/bukkit/plugin/PluginManagerTest.java b/src/test/java/org/bukkit/plugin/PluginManagerTest.java
|
2019-05-06 02:58:04 +00:00
|
|
|
index 17dbe913..bae26ce7 100644
|
2018-09-09 05:14:18 +00:00
|
|
|
--- a/src/test/java/org/bukkit/plugin/PluginManagerTest.java
|
|
|
|
+++ b/src/test/java/org/bukkit/plugin/PluginManagerTest.java
|
2019-04-23 04:47:07 +00:00
|
|
|
@@ -17,7 +17,7 @@ public class PluginManagerTest {
|
2018-09-09 05:14:18 +00:00
|
|
|
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);
|
2019-04-23 04:47:07 +00:00
|
|
|
@@ -28,14 +28,14 @@ public class PluginManagerTest {
|
2018-09-09 05:14:18 +00:00
|
|
|
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);
|
2019-05-06 02:58:04 +00:00
|
|
|
@@ -127,7 +127,7 @@ public class PluginManagerTest {
|
2019-04-23 04:47:07 +00:00
|
|
|
if (store.value == null) {
|
|
|
|
throw new IllegalStateException("No exception thrown");
|
|
|
|
}
|
2018-09-09 05:14:18 +00:00
|
|
|
- }
|
2019-04-23 04:47:07 +00:00
|
|
|
+ } */ // Paper
|
2018-09-09 05:14:18 +00:00
|
|
|
|
|
|
|
@Test
|
2019-04-23 04:47:07 +00:00
|
|
|
public void testRemovePermissionByNameLower() {
|
2018-09-09 05:04:29 +00:00
|
|
|
--
|
2019-03-20 00:28:15 +00:00
|
|
|
2.21.0
|
2018-09-09 05:04:29 +00:00
|
|
|
|