mirrors/papermc
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

Remove deadlock risk in firing async events

Browse source 
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
This commit is contained in:
Aikar 2018-09-09 01:04:29 -04:00
parent 4e30b91d4e
commit 5228a4f24c
No known key found for this signature in database
GPG key ID: 401ADFC9891FAAFE
6 changed files with 120 additions and 37 deletions
Download patch file Download diff file Expand all files Collapse all files

6
Spigot-API-Patches/0130-isChunkGenerated-API.patch → Spigot-API-Patches/0131-isChunkGenerated-API.patch
View file

@ -1,4 +1,4 @@
From b036cb38ebab110e948775363f38ffb16c149199 Mon Sep 17 00:00:00 2001
From 83e63475155067753ed63d35b7e5998c75feff72 Mon Sep 17 00:00:00 2001
From: cswhite2000 <18whitechristop@gmail.com>
Date: Tue, 21 Aug 2018 19:39:46 -0700
Subject: [PATCH] isChunkGenerated API
@ -6,7 +6,7 @@ Subject: [PATCH] isChunkGenerated API
Resolves #1329
diff --git a/src/main/java/org/bukkit/Location.java b/src/main/java/org/bukkit/Location.java
index 7e1ee875..9457832b 100644
index 7e1ee875e..9457832bc 100644
--- a/src/main/java/org/bukkit/Location.java
+++ b/src/main/java/org/bukkit/Location.java
@@ -9,6 +9,7 @@ import org.bukkit.util.NumberConversions;
@ -34,7 +34,7 @@ index 7e1ee875..9457832b 100644
/**
* Sets the position of this Location and returns itself
diff --git a/src/main/java/org/bukkit/World.java b/src/main/java/org/bukkit/World.java
index a6facc4b..d5058634 100644
index a6facc4b0..d50586349 100644
--- a/src/main/java/org/bukkit/World.java
+++ b/src/main/java/org/bukkit/World.java
@@ -210,6 +210,26 @@ public interface World extends PluginMessageRecipient, Metadatable {

6
Spigot-API-Patches/0131-Add-source-block-to-BlockPhysicsEvent.patch → Spigot-API-Patches/0132-Add-source-block-to-BlockPhysicsEvent.patch
View file

@ -1,11 +1,11 @@
From a802db6737065697d3668b53a32d9eb5265fdab5 Mon Sep 17 00:00:00 2001
From 3dfdb2062b0760e342784a09056d39c3266df738 Mon Sep 17 00:00:00 2001
From: Sotr <i@omc.hk>
Date: Thu, 23 Aug 2018 16:14:25 +0800
Subject: [PATCH] Add source block to BlockPhysicsEvent
diff --git a/src/main/java/org/bukkit/event/block/BlockPhysicsEvent.java b/src/main/java/org/bukkit/event/block/BlockPhysicsEvent.java
index 01a545b4..d17e05ac 100644
index 01a545b42..57568cd02 100644
--- a/src/main/java/org/bukkit/event/block/BlockPhysicsEvent.java
+++ b/src/main/java/org/bukkit/event/block/BlockPhysicsEvent.java
@@ -12,6 +12,39 @@ public class BlockPhysicsEvent extends BlockEvent implements Cancellable {
@ -57,5 +57,5 @@ index 01a545b4..d17e05ac 100644
/**
--
2.18.0.windows.1
2.18.0

83
Spigot-API-Patches/0133-Remove-deadlock-risk-in-firing-async-events.patch Normal file
View file

@ -0,0 +1,83 @@
From cc0de4b8d6d4373fada1806dc617c7dc71160094 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 cb2b0b9cb..a7dd902fb 100644
--- a/src/main/java/org/bukkit/plugin/SimplePluginManager.java
+++ b/src/main/java/org/bukkit/plugin/SimplePluginManager.java
@@ -385,7 +385,7 @@ public final class SimplePluginManager implements PluginManager {
* @param plugin Plugin to check
* @return true if the plugin is enabled, otherwise false
*/
- public boolean isPluginEnabled(Plugin plugin) {
+ public synchronized boolean isPluginEnabled(Plugin plugin) { // Paper - synchronize
if ((plugin != null) && (plugins.contains(plugin))) {
return plugin.isEnabled();
} else {
@@ -393,7 +393,7 @@ public final class SimplePluginManager implements PluginManager {
}
}
- public void enablePlugin(final Plugin plugin) {
+ public synchronized void enablePlugin(final Plugin plugin) { // Paper - synchronize
if (!plugin.isEnabled()) {
List<Command> pluginCommands = PluginCommandYamlParser.parse(plugin);
@@ -430,7 +430,7 @@ public final class SimplePluginManager implements PluginManager {
disablePlugin(plugin, false);
}
- public void disablePlugin(final Plugin plugin, boolean closeClassloader) {
+ public synchronized void disablePlugin(final Plugin plugin, boolean closeClassloader) { // Paper - synchronize
// Paper end - close Classloader on disable
if (plugin.isEnabled()) {
try {
@@ -490,6 +490,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.
@@ -499,22 +500,7 @@ public final class SimplePluginManager implements PluginManager {
* @param event Event details
*/
public void callEvent(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(Event event) {
+ // Paper - replace callEvent by merging to below method
HandlerList handlers = event.getHandlers();
RegisteredListener[] listeners = handlers.getRegisteredListeners();
--
2.18.0