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: 93e39ce1 Clarify documentation regarding getMaterial with legacyName = true c3aeaea0 Improve dependency tracker 14c9d275 Add support for transitive depends in load access warning c8afe560 SPIGOT-5526: Add EntityEnterBlockEvent 6bb6f07d SPIGOT-5548: Show error that hints towards plugins misusing reflection ed75537d SPIGOT-5546: Fix bad depend access using wrong provider in message 4e4c0ee9 Fix buggy classloader warning triggering for all classes 89586a4c Print warning when loading classes from depends that have not been specified d4fe9680 Fix bug where disablePlugin could remove ConfigurationSerializable classes from other plugins 85e683b7 Add additional checkstyle checks 612fd8e1 Correct max page count in BookMeta docs fa8a9781 Correct max title length in BookMeta docs CraftBukkit Changes: ab13a117 SPIGOT-5550: Cancelled ProjectileLaunchEvent still plays sound for eggs 44016b1d SPIGOT-5538: Using javaw to run GUI prints input error e653ae76 SPIGOT-5526: Call EntityEnterBlockEvent for bees trying to enter hives 6515ea49 SPIGOT-5537: Bee nests generated by growing trees near flower have no bees d82b3149 Remove unused CraftWorld.getId method 10763a88 Change some block == AIR checks to isAir to catch CAVE_AIR Spigot Changes: f2c1cd15 Rebuild patches bcd458ad Reformat patches
143 lines
5.9 KiB
Diff
143 lines
5.9 KiB
Diff
From e7fbc30984f676f22c203d4a1eff91ae30f9e9dc 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 a8dbf282..b4069dbf 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 7453230c..634e51d5 100644
|
|
--- a/src/main/java/org/bukkit/plugin/SimplePluginManager.java
|
|
+++ b/src/main/java/org/bukkit/plugin/SimplePluginManager.java
|
|
@@ -414,7 +414,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 {
|
|
@@ -423,7 +423,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);
|
|
|
|
@@ -461,7 +461,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 {
|
|
@@ -531,6 +531,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.
|
|
@@ -541,25 +542,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 f188cd4f..1941c9f4 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);
|
|
@@ -129,7 +129,7 @@ public class PluginManagerTest {
|
|
if (store.value == null) {
|
|
throw new IllegalStateException("No exception thrown");
|
|
}
|
|
- }
|
|
+ } */ // Paper
|
|
|
|
@Test
|
|
public void testRemovePermissionByNameLower() {
|
|
--
|
|
2.25.0
|
|
|