From 6707c4811e58aa253c038b39f26dc2080ead342c Mon Sep 17 00:00:00 2001 From: Pasqual Koschmieder Date: Wed, 17 Aug 2022 05:51:54 +0200 Subject: [PATCH] only run inbound listeners on the main thread if requested (#1851) --- .../protocol/events/ListenerOptions.java | 8 +++- .../protocol/events/PacketAdapter.java | 12 +++++- .../protocol/events/PacketListener.java | 38 ++++++++++--------- .../manager/NetworkManagerPacketInjector.java | 2 +- 4 files changed, 40 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/comphenix/protocol/events/ListenerOptions.java b/src/main/java/com/comphenix/protocol/events/ListenerOptions.java index 3233796b..72f7c8c7 100644 --- a/src/main/java/com/comphenix/protocol/events/ListenerOptions.java +++ b/src/main/java/com/comphenix/protocol/events/ListenerOptions.java @@ -23,5 +23,11 @@ public enum ListenerOptions { /** * Notify ProtocolLib that {@link PacketListener#onPacketSending(PacketEvent)} is thread safe. */ - ASYNC; + ASYNC, + + /** + * Notify ProtocolLib that {@link PacketListener#onPacketReceiving(PacketEvent)} must be executed on the main server + * thread. + */ + SYNC } diff --git a/src/main/java/com/comphenix/protocol/events/PacketAdapter.java b/src/main/java/com/comphenix/protocol/events/PacketAdapter.java index 977093a4..ca319f3c 100644 --- a/src/main/java/com/comphenix/protocol/events/PacketAdapter.java +++ b/src/main/java/com/comphenix/protocol/events/PacketAdapter.java @@ -405,7 +405,7 @@ public abstract class PacketAdapter implements PacketListener { } /** - * Set the listener option to {@link ListenerOptions#ASYNC}, indicating that our listener is thread safe. + * Set the listener option to {@link ListenerOptions#ASYNC}, indicating that our outbound listener is thread safe. *

* This allows ProtocolLib to perform certain optimizations. * @@ -415,6 +415,16 @@ public abstract class PacketAdapter implements PacketListener { return addOption(ListenerOptions.ASYNC); } + /** + * Set the listener option to {@link ListenerOptions#SYNC}, indicating that our inbound listener must be executed + * on the main server thread. + * + * @return This builder, for chaining. + */ + public AdapterParameteters optionSync() { + return addOption(ListenerOptions.SYNC); + } + /** * Set the packet types the listener is looking for. *

diff --git a/src/main/java/com/comphenix/protocol/events/PacketListener.java b/src/main/java/com/comphenix/protocol/events/PacketListener.java index 7a83d4f7..9a1c94e5 100644 --- a/src/main/java/com/comphenix/protocol/events/PacketListener.java +++ b/src/main/java/com/comphenix/protocol/events/PacketListener.java @@ -23,49 +23,53 @@ import org.bukkit.plugin.Plugin; * Represents a listener that receives notifications when packets are sending or being received. *

* Use {@link PacketAdapter} for a simple wrapper around this interface. + * * @author Kristian */ public interface PacketListener { - + /** * Invoked right before a packet is transmitted from the server to the client. *

* Note that the packet may be replaced, if needed. *

- * This method is executed on the main thread in 1.6.4 and earlier, and thus the Bukkit API is safe to use. - *

- * In Minecraft 1.7.2 and later, this method MAY be executed asynchronously, but only if {@link ListenerOptions#ASYNC} - * have been specified in the listener. This is off by default. + * This method is executed on the main server thread by default. However, some spigot forks (like paper) schedule + * specific packets off the main thread. If the {@link ListenerOptions#ASYNC} option is not specified any invocation + * of this method will be on the main server thread. + * * @param event - the packet that should be sent. */ - public void onPacketSending(PacketEvent event); + void onPacketSending(PacketEvent event); /** * Invoked right before a received packet from a client is being processed. *

- * WARNING:
- * This method will be called asynchronously! You should synchronize with the main - * thread using {@link org.bukkit.scheduler.BukkitScheduler#scheduleSyncDelayedTask(Plugin, Runnable, long) scheduleSyncDelayedTask} - * if you need to call the Bukkit API. + * This method will be called asynchronously (or on the netty event loop) by default. If the + * {@link ListenerOptions#SYNC} option is specified, the invocation of this method will be synced to the main server + * thread which might cause issues due to delayed packets. + * * @param event - the packet that has been received. */ - public void onPacketReceiving(PacketEvent event); - + void onPacketReceiving(PacketEvent event); + /** * Retrieve which packets sent by the server this listener will observe. + * * @return List of server packets to observe, along with the priority. */ - public ListeningWhitelist getSendingWhitelist(); - + ListeningWhitelist getSendingWhitelist(); + /** * Retrieve which packets sent by the client this listener will observe. + * * @return List of server packets to observe, along with the priority. */ - public ListeningWhitelist getReceivingWhitelist(); - + ListeningWhitelist getReceivingWhitelist(); + /** * Retrieve the plugin that created list packet listener. + * * @return The plugin, or NULL if not available. */ - public Plugin getPlugin(); + Plugin getPlugin(); } diff --git a/src/main/java/com/comphenix/protocol/injector/netty/manager/NetworkManagerPacketInjector.java b/src/main/java/com/comphenix/protocol/injector/netty/manager/NetworkManagerPacketInjector.java index 2cc75452..d8ccb5a0 100644 --- a/src/main/java/com/comphenix/protocol/injector/netty/manager/NetworkManagerPacketInjector.java +++ b/src/main/java/com/comphenix/protocol/injector/netty/manager/NetworkManagerPacketInjector.java @@ -33,7 +33,7 @@ final class NetworkManagerPacketInjector extends AbstractPacketInjector { @Override public boolean addPacketHandler(PacketType type, Set options) { - if (!type.isAsyncForced() && (options == null || !options.contains(ListenerOptions.ASYNC))) { + if (!type.isAsyncForced() && options != null && options.contains(ListenerOptions.SYNC)) { this.mainThreadListeners.addType(type); }