mirror of
https://github.com/PaperMC/Paper.git
synced 2024-09-06 03:48:05 +02:00
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 Bukkit Changes: e99c9444 Add Plugin Chunk Ticket API 6a235f06 Fix incorrect nullability annotations for PlayerJoinEvent's join message CraftBukkit Changes:5f889388
Tweak build expiration to 7 days572c02b0
MC-155077, SPIGOT-5113: EntityTracker desync7ad3a1f4
SPIGOT-5146: BlockDataMeta does not work60860983
SPIGOT-5155: Setting EntityExplodeEvent yield to 0 still causes blocks to drop087a2cf4
Print number of force loaded chunks per plugin in crash reports07b5b06d
Add Plugin Chunk Ticket API7ffb2a27
SPIGOT-5149: resetRecipes does nothinga2275f19
SPIGOT-5141: World.generateTree() causes ClassCastException with huge mushrooms31d4a777
SPIGOT-5142: Ignore invalid firework effects Spigot Changes: 5e4e7f32 BUILDTOOLS-471: Rebuild patches 6e944739 SPIGOT-5159: Raider activation range overridden by Monster range
143 lines
5.9 KiB
Diff
143 lines
5.9 KiB
Diff
From e724746a98cf149f377d9a442782dc448abb8405 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/entity/Entity.java b/src/main/java/org/bukkit/entity/Entity.java
|
|
index adca48f1..2d3cee14 100644
|
|
--- a/src/main/java/org/bukkit/entity/Entity.java
|
|
+++ b/src/main/java/org/bukkit/entity/Entity.java
|
|
@@ -28,7 +28,7 @@ import org.jetbrains.annotations.Nullable;
|
|
*/
|
|
public interface Entity extends Metadatable, CommandSender, Nameable, PersistentDataHolder {
|
|
|
|
- /**
|
|
+ /*
|
|
* Gets the entity's current position
|
|
*
|
|
* @return a new copy of Location containing the position of this entity
|
|
diff --git a/src/main/java/org/bukkit/plugin/SimplePluginManager.java b/src/main/java/org/bukkit/plugin/SimplePluginManager.java
|
|
index 132c861c..d0e735bc 100644
|
|
--- a/src/main/java/org/bukkit/plugin/SimplePluginManager.java
|
|
+++ b/src/main/java/org/bukkit/plugin/SimplePluginManager.java
|
|
@@ -399,7 +399,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 {
|
|
@@ -408,7 +408,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);
|
|
|
|
@@ -446,7 +446,7 @@ public final class SimplePluginManager implements PluginManager {
|
|
}
|
|
|
|
@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
|
|
if (plugin.isEnabled()) {
|
|
try {
|
|
@@ -515,6 +515,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.
|
|
@@ -525,25 +526,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.");
|
|
- }
|
|
- fireEvent(event);
|
|
- } else {
|
|
- if (!server.isPrimaryThread()) {
|
|
- throw new IllegalStateException(event.getEventName() + " cannot be triggered asynchronously from another thread.");
|
|
- }
|
|
- synchronized (this) {
|
|
- fireEvent(event);
|
|
- }
|
|
+ // 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.");
|
|
}
|
|
- }
|
|
|
|
- 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 17dbe913..bae26ce7 100644
|
|
--- a/src/test/java/org/bukkit/plugin/PluginManagerTest.java
|
|
+++ b/src/test/java/org/bukkit/plugin/PluginManagerTest.java
|
|
@@ -17,7 +17,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);
|
|
@@ -28,14 +28,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);
|
|
@@ -127,7 +127,7 @@ public class PluginManagerTest {
|
|
if (store.value == null) {
|
|
throw new IllegalStateException("No exception thrown");
|
|
}
|
|
- }
|
|
+ } */ // Paper
|
|
|
|
@Test
|
|
public void testRemovePermissionByNameLower() {
|
|
--
|
|
2.22.0
|
|
|