PaperMC/Spigot-API-Patches/0129-Remove-deadlock-risk-in-firing-async-events.patch
Spottedleaf 5c7081fecc Update upstream & fix some chunk related issues (#2177)
* Updated Upstream (Bukkit/CraftBukkit)

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:
45690fe9 SPIGOT-5047: Correct slot types for 1.14 inventories

CraftBukkit Changes:
4090d01f SPIGOT-5047: Correct slot types for 1.14 inventories
e8c08362 SPIGOT-5046: World#getLoadedChunks returning inaccessible cached chunks.
d445af3b SPIGOT-5067: Add item meta for 1.14 spawn eggs

* Bring Chunk load checks in-line with spigot

As of the last upstream merge spigot now checks ticket level status
when returning loaded chunks for a world from api. Now our checks
will respect that decision.

* Fix spawn ticket levels

Vanilla would keep the inner chunks of spawn available for ticking,
however my changes made all chunks non-ticking. Resolve by changing
ticket levels for spawn chunks inside the border to respect this
behavior.


* Make World#getChunkIfLoadedImmediately return only entity ticking chunks

Mojang appears to be using chunks with level > 33 (non-ticking chunks)
as cached chunks and not actually loaded chunks.

* Bring all loaded checks in line with spigot

Loaded chunks must be at least border  chunks, or level <= 33
2019-06-14 03:27:40 +01:00

142 lines
5.9 KiB
Diff

From 47329276123baf9d077f10ba1686f54e85ef861f 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 adca48f18..2d3cee140 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 da3cd63ba..5eefb4b5b 100644
--- a/src/main/java/org/bukkit/plugin/SimplePluginManager.java
+++ b/src/main/java/org/bukkit/plugin/SimplePluginManager.java
@@ -398,7 +398,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 {
@@ -407,7 +407,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);
@@ -445,7 +445,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 {
@@ -506,6 +506,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.
@@ -516,25 +517,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 17dbe9139..bae26ce73 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.21.0