Support asynchronous events; Addresses BUKKIT-1212

By: Wesley Wolfe <weswolf@aol.com>

paper-api/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();
+    }
 }

paper-api/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();
 }

paper-api/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 {
 
         /**

paper-api/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,22 +33,28 @@ public class HandlerList {
      * you're using fevents in a plugin system.
      */
     public static void bakeAll() {
+        synchronized (allLists) {
             for (HandlerList h : allLists) {
                 h.bake();
             }
         }
+    }
 
     /**
      * Unregister all listeners from all handler lists.
      */
     public static void unregisterAll() {
+        synchronized (allLists) {
             for (HandlerList h : allLists) {
+                synchronized (h) {
                     for (List<RegisteredListener> list : h.handlerslots.values()) {
                         list.clear();
                     }
                     h.handlers = null;
                 }
             }
+        }
+    }
 
     /**
      * Unregister a specific plugin's listeners from all handler lists.
@@ -56,10 +62,12 @@ public class HandlerList {
      * @param plugin plugin to unregister
      */
     public static void unregisterAll(Plugin plugin) {
+        synchronized (allLists) {
             for (HandlerList h : allLists) {
                 h.unregister(plugin);
             }
         }
+    }
 
     /**
      * Unregister a specific listener from all handler lists.
@@ -67,10 +75,12 @@ public class HandlerList {
      * @param listener listener to unregister
      */
     public static void unregisterAll(Listener listener) {
+        synchronized (allLists) {
             for (HandlerList h : allLists) {
                 h.unregister(listener);
             }
         }
+    }
 
     /**
      * Create a new handler list and initialize using EventPriority
@@ -81,15 +91,17 @@ public class HandlerList {
         for (EventPriority o : EventPriority.values()) {
             handlerslots.put(o, new ArrayList<RegisteredListener>());
         }
+        synchronized (allLists) {
             allLists.add(this);
         }
+    }
 
     /**
      * Register a new listener in this handler list
      *
      * @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,7 +198,9 @@ public class HandlerList {
      */
     public static ArrayList<RegisteredListener> getRegisteredListeners(Plugin plugin) {
         ArrayList<RegisteredListener> listeners = new ArrayList<RegisteredListener>();
+        synchronized (allLists) {
             for (HandlerList h : allLists) {
+                synchronized (h) {
                     for (List<RegisteredListener> list : h.handlerslots.values()) {
                         for (RegisteredListener listener : list) {
                             if (listener.getPlugin().equals(plugin)) {
@@ -194,6 +209,8 @@ public class HandlerList {
                         }
                     }
                 }
+            }
+        }
         return listeners;
     }
 
@@ -204,6 +221,8 @@ public class HandlerList {
      */
     @SuppressWarnings("unchecked")
     public static ArrayList<HandlerList> getHandlerLists() {
+        synchronized (allLists) {
             return (ArrayList<HandlerList>) allLists.clone();
         }
+    }
 }

paper-api/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

paper-api/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();
 

paper-api/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;

paper-api/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));
+    }
+}

paper-api/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;
+    }
+}

paper-api/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);
+        }
+    }
+}