summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--src/main/java/org/bukkit/Bukkit.java4
-rw-r--r--src/main/java/org/bukkit/Server.java5
-rw-r--r--src/main/java/org/bukkit/event/Event.java35
-rw-r--r--src/main/java/org/bukkit/event/HandlerList.java67
-rw-r--r--src/main/java/org/bukkit/plugin/PluginManager.java5
-rw-r--r--src/main/java/org/bukkit/plugin/SimplePluginManager.java21
-rw-r--r--src/main/java/org/bukkit/plugin/TimedRegisteredListener.java5
-rw-r--r--src/test/java/org/bukkit/TestServer.java48
-rw-r--r--src/test/java/org/bukkit/event/TestEvent.java19
-rw-r--r--src/test/java/org/bukkit/plugin/PluginManagerTest.java118
10 files changed, 300 insertions, 27 deletions
diff --git a/src/main/java/org/bukkit/Bukkit.java b/src/main/java/org/bukkit/Bukkit.java
index 8654b50a..f59d4342 100644
--- a/src/main/java/org/bukkit/Bukkit.java
+++ b/src/main/java/org/bukkit/Bukkit.java
@@ -365,4 +365,8 @@ public final class Bukkit {
public static int getWaterAnimalSpawnLimit() {
return server.getWaterAnimalSpawnLimit();
}
+
+ public static boolean isPrimaryThread() {
+ return server.isPrimaryThread();
+ }
}
diff --git a/src/main/java/org/bukkit/Server.java b/src/main/java/org/bukkit/Server.java
index 9377d0cd..a2b3ff9c 100644
--- a/src/main/java/org/bukkit/Server.java
+++ b/src/main/java/org/bukkit/Server.java
@@ -639,4 +639,9 @@ public interface Server extends PluginMessageRecipient {
* @returns The water animal spawn limit
*/
int getWaterAnimalSpawnLimit();
+
+ /**
+ * Returns true if the current {@link Thread} is the server's primary thread
+ */
+ boolean isPrimaryThread();
} \ No newline at end of file
diff --git a/src/main/java/org/bukkit/event/Event.java b/src/main/java/org/bukkit/event/Event.java
index 92351af2..273e5b26 100644
--- a/src/main/java/org/bukkit/event/Event.java
+++ b/src/main/java/org/bukkit/event/Event.java
@@ -5,6 +5,23 @@ package org.bukkit.event;
*/
public abstract class Event {
private String name;
+ private final boolean async;
+
+ /**
+ * The default constructor is defined for cleaner code.
+ * This constructor assumes the event is synchronous.
+ */
+ public Event() {
+ this(false);
+ }
+
+ /**
+ * This constructor is used to explicitly declare an event as synchronous or asynchronous.
+ * @param isAsync true indicates the event will fire asynchronously. false by default
+ */
+ public Event(boolean isAsync) {
+ this.async = isAsync;
+ }
/**
* @return Name of this event
@@ -18,6 +35,24 @@ public abstract class Event {
public abstract HandlerList getHandlers();
+ /**
+ * Any custom event that should not by synchronized with other events must use the specific constructor.
+ * These are the caveats of using an asynchronous event:
+ * <li>The event is never fired from inside code triggered by a synchronous event.
+ * Attempting to do so results in an {@link java.lang.IllegalStateException}.</li>
+ * <li>However, asynchronous event handlers may fire synchronous or asynchronous events</li>
+ * <li>The event may be fired multiple times simultaneously and in any order.</li>
+ * <li>Any newly registered or unregistered handler is ignored after an event starts execution.</li>
+ * <li>The handlers for this event may block for any length of time.</li>
+ * <li>Some implementations may selectively declare a specific event use as asynchronous.
+ * This behavior should be clearly defined.</li>
+ * <li>Asynchronous calls are not calculated in the plugin timing system.</li>
+ * @return false by default, true if the event fires asynchronously
+ */
+ public final boolean isAsynchronous() {
+ return async;
+ }
+
public enum Result {
/**
diff --git a/src/main/java/org/bukkit/event/HandlerList.java b/src/main/java/org/bukkit/event/HandlerList.java
index f48d942f..b0014119 100644
--- a/src/main/java/org/bukkit/event/HandlerList.java
+++ b/src/main/java/org/bukkit/event/HandlerList.java
@@ -13,7 +13,7 @@ public class HandlerList {
/**
* Handler array. This field being an array is the key to this system's speed.
*/
- private RegisteredListener[] handlers = null;
+ private volatile RegisteredListener[] handlers = null;
/**
* Dynamic handler lists. These are changed using register() and
@@ -33,8 +33,10 @@ public class HandlerList {
* you're using fevents in a plugin system.
*/
public static void bakeAll() {
- for (HandlerList h : allLists) {
- h.bake();
+ synchronized (allLists) {
+ for (HandlerList h : allLists) {
+ h.bake();
+ }
}
}
@@ -42,11 +44,15 @@ public class HandlerList {
* Unregister all listeners from all handler lists.
*/
public static void unregisterAll() {
- for (HandlerList h : allLists) {
- for (List<RegisteredListener> list : h.handlerslots.values()) {
- list.clear();
+ synchronized (allLists) {
+ for (HandlerList h : allLists) {
+ synchronized (h) {
+ for (List<RegisteredListener> list : h.handlerslots.values()) {
+ list.clear();
+ }
+ h.handlers = null;
+ }
}
- h.handlers = null;
}
}
@@ -56,8 +62,10 @@ public class HandlerList {
* @param plugin plugin to unregister
*/
public static void unregisterAll(Plugin plugin) {
- for (HandlerList h : allLists) {
- h.unregister(plugin);
+ synchronized (allLists) {
+ for (HandlerList h : allLists) {
+ h.unregister(plugin);
+ }
}
}
@@ -67,8 +75,10 @@ public class HandlerList {
* @param listener listener to unregister
*/
public static void unregisterAll(Listener listener) {
- for (HandlerList h : allLists) {
- h.unregister(listener);
+ synchronized (allLists) {
+ for (HandlerList h : allLists) {
+ h.unregister(listener);
+ }
}
}
@@ -81,7 +91,9 @@ public class HandlerList {
for (EventPriority o : EventPriority.values()) {
handlerslots.put(o, new ArrayList<RegisteredListener>());
}
- allLists.add(this);
+ synchronized (allLists) {
+ allLists.add(this);
+ }
}
/**
@@ -89,7 +101,7 @@ public class HandlerList {
*
* @param listener listener to register
*/
- public void register(RegisteredListener listener) {
+ public synchronized void register(RegisteredListener listener) {
if (handlerslots.get(listener.getPriority()).contains(listener))
throw new IllegalStateException("This listener is already registered to priority " + listener.getPriority().toString());
handlers = null;
@@ -112,7 +124,7 @@ public class HandlerList {
*
* @param listener listener to remove
*/
- public void unregister(RegisteredListener listener) {
+ public synchronized void unregister(RegisteredListener listener) {
if (handlerslots.get(listener.getPriority()).remove(listener)) {
handlers = null;
}
@@ -123,7 +135,7 @@ public class HandlerList {
*
* @param plugin plugin to remove
*/
- public void unregister(Plugin plugin) {
+ public synchronized void unregister(Plugin plugin) {
boolean changed = false;
for (List<RegisteredListener> list : handlerslots.values()) {
for (ListIterator<RegisteredListener> i = list.listIterator(); i.hasNext();) {
@@ -141,7 +153,7 @@ public class HandlerList {
*
* @param listener listener to remove
*/
- public void unregister(Listener listener) {
+ public synchronized void unregister(Listener listener) {
boolean changed = false;
for (List<RegisteredListener> list : handlerslots.values()) {
for (ListIterator<RegisteredListener> i = list.listIterator(); i.hasNext();) {
@@ -157,7 +169,7 @@ public class HandlerList {
/**
* Bake HashMap and ArrayLists to 2d array - does nothing if not necessary
*/
- public void bake() {
+ public synchronized void bake() {
if (handlers != null) return; // don't re-bake when still valid
List<RegisteredListener> entries = new ArrayList<RegisteredListener>();
for (Entry<EventPriority, ArrayList<RegisteredListener>> entry : handlerslots.entrySet()) {
@@ -172,7 +184,8 @@ public class HandlerList {
* @return the array of registered listeners
*/
public RegisteredListener[] getRegisteredListeners() {
- bake();
+ RegisteredListener[] handlers;
+ while ((handlers = this.handlers) == null) bake(); // This prevents fringe cases of returning null
return handlers;
}
@@ -185,11 +198,15 @@ public class HandlerList {
*/
public static ArrayList<RegisteredListener> getRegisteredListeners(Plugin plugin) {
ArrayList<RegisteredListener> listeners = new ArrayList<RegisteredListener>();
- for (HandlerList h : allLists) {
- for (List<RegisteredListener> list : h.handlerslots.values()) {
- for (RegisteredListener listener : list) {
- if (listener.getPlugin().equals(plugin)) {
- listeners.add(listener);
+ synchronized (allLists) {
+ for (HandlerList h : allLists) {
+ synchronized (h) {
+ for (List<RegisteredListener> list : h.handlerslots.values()) {
+ for (RegisteredListener listener : list) {
+ if (listener.getPlugin().equals(plugin)) {
+ listeners.add(listener);
+ }
+ }
}
}
}
@@ -204,6 +221,8 @@ public class HandlerList {
*/
@SuppressWarnings("unchecked")
public static ArrayList<HandlerList> getHandlerLists() {
- return (ArrayList<HandlerList>) allLists.clone();
+ synchronized (allLists) {
+ return (ArrayList<HandlerList>) allLists.clone();
+ }
}
}
diff --git a/src/main/java/org/bukkit/plugin/PluginManager.java b/src/main/java/org/bukkit/plugin/PluginManager.java
index 804f4415..89697de8 100644
--- a/src/main/java/org/bukkit/plugin/PluginManager.java
+++ b/src/main/java/org/bukkit/plugin/PluginManager.java
@@ -92,8 +92,11 @@ public interface PluginManager {
* Calls an event with the given details
*
* @param event Event details
+ * @throws IllegalStateException Thrown when an asynchronous event is fired from synchronous code.<br>
+ * <i>Note: This is best-effort basis, and should not be used to test synchronized state. This
+ * is an indicator for flawed flow logic.</i>
*/
- public void callEvent(Event event);
+ public void callEvent(Event event) throws IllegalStateException;
/**
* Registers all the events in the given listener class
diff --git a/src/main/java/org/bukkit/plugin/SimplePluginManager.java b/src/main/java/org/bukkit/plugin/SimplePluginManager.java
index e017ffdf..ca780fca 100644
--- a/src/main/java/org/bukkit/plugin/SimplePluginManager.java
+++ b/src/main/java/org/bukkit/plugin/SimplePluginManager.java
@@ -443,11 +443,28 @@ public final class SimplePluginManager implements PluginManager {
}
/**
- * Calls an event with the given details
+ * Calls an event with the given details.<br>
+ * This method only synchronizes when the event is not asynchronous.
*
* @param event Event details
*/
- public synchronized void callEvent(Event event) {
+ 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) {
HandlerList handlers = event.getHandlers();
RegisteredListener[] listeners = handlers.getRegisteredListeners();
diff --git a/src/main/java/org/bukkit/plugin/TimedRegisteredListener.java b/src/main/java/org/bukkit/plugin/TimedRegisteredListener.java
index 53dfde38..ed25e179 100644
--- a/src/main/java/org/bukkit/plugin/TimedRegisteredListener.java
+++ b/src/main/java/org/bukkit/plugin/TimedRegisteredListener.java
@@ -18,7 +18,12 @@ public class TimedRegisteredListener extends RegisteredListener {
super(pluginListener, eventExecutor, eventPriority, registeredPlugin, listenCancelled);
}
+ @Override
public void callEvent(Event event) throws EventException {
+ if (event.isAsynchronous()) {
+ super.callEvent(event);
+ return;
+ }
count++;
if (this.event == null) {
this.event = event;
diff --git a/src/test/java/org/bukkit/TestServer.java b/src/test/java/org/bukkit/TestServer.java
new file mode 100644
index 00000000..63e8aeff
--- /dev/null
+++ b/src/test/java/org/bukkit/TestServer.java
@@ -0,0 +1,48 @@
+package org.bukkit;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
+import java.util.HashMap;
+
+
+public class TestServer implements InvocationHandler {
+ private static interface MethodHandler {
+ Object handle(TestServer server, Object[] args);
+ }
+ private static final Constructor<? extends Server> constructor;
+ private static final HashMap<Method, MethodHandler> methods = new HashMap<Method, MethodHandler>();
+ static {
+ try {
+ methods.put(Server.class.getMethod("isPrimaryThread"),
+ new MethodHandler() {
+ public Object handle(TestServer server, Object[] args) {
+ return Thread.currentThread().equals(server.creatingThread);
+ }
+ });
+ constructor = Proxy.getProxyClass(Server.class.getClassLoader(), Server.class).asSubclass(Server.class).getConstructor(InvocationHandler.class);
+ } catch (Throwable t) {
+ throw new Error(t);
+ }
+ }
+
+ private Thread creatingThread = Thread.currentThread();
+ private TestServer() {};
+
+ public static Server getInstance() {
+ try {
+ return constructor.newInstance(new TestServer());
+ } catch (Throwable t) {
+ throw new RuntimeException(t);
+ }
+ }
+
+ public Object invoke(Object proxy, Method method, Object[] args) {
+ MethodHandler handler = methods.get(method);
+ if (handler != null) {
+ return handler.handle(this, args);
+ }
+ throw new UnsupportedOperationException(String.valueOf(method));
+ }
+}
diff --git a/src/test/java/org/bukkit/event/TestEvent.java b/src/test/java/org/bukkit/event/TestEvent.java
new file mode 100644
index 00000000..c06dfe94
--- /dev/null
+++ b/src/test/java/org/bukkit/event/TestEvent.java
@@ -0,0 +1,19 @@
+package org.bukkit.event;
+
+
+public class TestEvent extends Event {
+ private static final HandlerList handlers = new HandlerList();
+
+ public TestEvent(boolean async) {
+ super(async);
+ }
+
+ @Override
+ public HandlerList getHandlers() {
+ return handlers;
+ }
+
+ public static HandlerList getHandlerList() {
+ return handlers;
+ }
+}
diff --git a/src/test/java/org/bukkit/plugin/PluginManagerTest.java b/src/test/java/org/bukkit/plugin/PluginManagerTest.java
new file mode 100644
index 00000000..b5cb7408
--- /dev/null
+++ b/src/test/java/org/bukkit/plugin/PluginManagerTest.java
@@ -0,0 +1,118 @@
+package org.bukkit.plugin;
+
+import static junit.framework.Assert.*;
+
+import org.bukkit.Server;
+import org.bukkit.TestServer;
+import org.bukkit.command.SimpleCommandMap;
+import org.bukkit.event.Event;
+import org.bukkit.event.TestEvent;
+import org.junit.Test;
+
+public class PluginManagerTest {
+ private class MutableObject {
+ volatile Object value = null;
+ }
+
+ final Server server = TestServer.getInstance();
+ final SimpleCommandMap commandMap = new SimpleCommandMap(server);
+ final PluginManager pm = new SimplePluginManager(server, commandMap);
+ final MutableObject store = new MutableObject();
+
+ @Test
+ public void testAsyncSameThread() {
+ final Event event = new TestEvent(true);
+ try {
+ pm.callEvent(event);
+ } catch (IllegalStateException ex) {
+ assertEquals(event.getEventName() + " cannot be triggered asynchronously from primary server thread.", ex.getMessage());
+ return;
+ }
+ throw new IllegalStateException("No exception thrown");
+ }
+
+ @Test
+ public void testSyncSameThread() {
+ final Event event = new TestEvent(false);
+ pm.callEvent(event);
+ }
+
+ @Test
+ public void testAsyncLocked() throws InterruptedException {
+ final Event event = new TestEvent(true);
+ Thread secondThread = new Thread(
+ new Runnable() {
+ public void run() {
+ try {
+ synchronized (pm) {
+ pm.callEvent(event);
+ }
+ } catch (Throwable ex) {
+ store.value = ex;
+ }
+ }});
+ secondThread.start();
+ secondThread.join();
+ assertTrue(store.value instanceof IllegalStateException);
+ assertEquals(event.getEventName() + " cannot be triggered asynchronously from inside synchronized code.", ((Throwable) store.value).getMessage());
+ }
+
+ @Test
+ public void testAsyncUnlocked() throws InterruptedException {
+ final Event event = new TestEvent(true);
+ Thread secondThread = new Thread(
+ new Runnable() {
+ public void run() {
+ try {
+ pm.callEvent(event);
+ } catch (Throwable ex) {
+ store.value = ex;
+ }
+ }});
+ secondThread.start();
+ secondThread.join();
+ if (store.value != null) {
+ throw new RuntimeException((Throwable) store.value);
+ }
+ }
+
+ @Test
+ public void testSyncUnlocked() throws InterruptedException {
+ final Event event = new TestEvent(false);
+ Thread secondThread = new Thread(
+ new Runnable() {
+ public void run() {
+ try {
+ pm.callEvent(event);
+ } catch (Throwable ex) {
+ store.value = ex;
+ }
+ }});
+ secondThread.start();
+ secondThread.join();
+ if (store.value != null) {
+ throw new RuntimeException((Throwable) store.value);
+ }
+ }
+
+ @Test
+ public void testSyncLocked() throws InterruptedException {
+ final Event event = new TestEvent(false);
+ Thread secondThread = new Thread(
+ new Runnable() {
+ public void run() {
+ try {
+ synchronized (pm) {
+ pm.callEvent(event);
+ }
+ } catch (Throwable ex) {
+ store.value = ex;
+ }
+ }});
+ secondThread.start();
+ secondThread.join();
+ if (store.value != null) {
+ throw new RuntimeException((Throwable) store.value);
+ }
+ }
+}