Fix server pings and the infamous closed channel exception

As a result, packet listeners for OUT_SERVER_INFO will be processed on
the netty server io thread, so they will have to be thread safe if they
aren't already.
Fixes #119

Author: dmulloy2

ProtocolLib/src/main/java/com/comphenix/protocol/PacketType.java

@@ -269,7 +269,7 @@ public static class Status {
 		public static class Server extends ObjectEnum<PacketType> {
 			private final static Sender SENDER = Sender.SERVER;
 
-			public static final PacketType OUT_SERVER_INFO =          new PacketType(PROTOCOL, SENDER, 0x00, 255);
+			public static final PacketType OUT_SERVER_INFO =          new PacketType(PROTOCOL, SENDER, 0x00, 255).forceAsync(true);
 			public static final PacketType OUT_PING =                 new PacketType(PROTOCOL, SENDER, 0x01, 230);
 
 			private final static Server INSTANCE = new Server();
@@ -523,7 +523,8 @@ public ConnectionSide toSide() {
 	private final int currentId;
 	private final int legacyId;
 	private final MinecraftVersion version;
-	private final boolean dynamic;
+	private boolean forceAsync;
+	private boolean dynamic;
 
 	/**
 	 * Retrieve the current packet/legacy lookup.
@@ -679,7 +680,8 @@ public static PacketType fromCurrent(Protocol protocol, Sender sender, int packe
 		PacketType type = getLookup().getFromCurrent(protocol, sender, packetId);
 
 		if (type == null) {
-			type = new PacketType(protocol, sender, packetId, legacyId, PROTOCOL_VERSION, true);
+			type = new PacketType(protocol, sender, packetId, legacyId, PROTOCOL_VERSION);
+			type.dynamic = true;
 
 			// Many may be scheduled, but only the first will be executed
 			scheduleRegister(type, "Dynamic-" + UUID.randomUUID().toString());
@@ -798,25 +800,11 @@ public PacketType(Protocol protocol, Sender sender, int currentId, int legacyId)
 	 * @param version - the version of the current ID.
 	 */
 	public PacketType(Protocol protocol, Sender sender, int currentId, int legacyId, MinecraftVersion version) {
-		this(protocol, sender, currentId, legacyId, version, false);
-	}
-
-	/**
-	 * Construct a new packet type.
-	 * @param protocol - the current protocol.
-	 * @param sender - client or server.
-	 * @param currentId - the current packet ID.
-	 * @param legacyId - the legacy packet ID.
-	 * @param version - the version of the current ID.
-	 * @param dynamic - if this type was created dynamically.
-	 */
-	public PacketType(Protocol protocol, Sender sender, int currentId, int legacyId, MinecraftVersion version, boolean dynamic) {
 		this.protocol = Preconditions.checkNotNull(protocol, "protocol cannot be NULL");
 		this.sender = Preconditions.checkNotNull(sender, "sender cannot be NULL");
 		this.currentId = currentId;
 		this.legacyId = legacyId;
 		this.version = version;
-		this.dynamic = dynamic;
 	}
 
 	/**
@@ -920,13 +908,26 @@ public int getLegacyId() {
 	}
 
 	/**
-	 * Whether or not this packet was dynamically created (ie we don't have it registered)
+	 * Whether or not this packet was dynamically created (i.e. we don't have it registered)
 	 * @return True if dnyamic, false if not.
 	 */
 	public boolean isDynamic() {
 		return dynamic;
 	}
 
+	private PacketType forceAsync(boolean forceAsync) {
+		this.forceAsync = forceAsync;
+		return this;
+	}
+
+	/**
+	 * Whether or not this packet must be processed asynchronously.
+	 * @return True if it must be, false if not.
+	 */
+	public boolean forceAsync() {
+		return forceAsync;
+	}
+
 	@Override
 	public int hashCode() {
 		return Objects.hashCode(protocol, sender, currentId, legacyId);

ProtocolLib/src/main/java/com/comphenix/protocol/compat/netty/independent/NettyChannelInjector.java

@@ -19,22 +19,19 @@
 import io.netty.buffer.ByteBuf;
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelHandler;
-import io.netty.channel.ChannelHandlerAdapter;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelInboundHandlerAdapter;
 import io.netty.channel.ChannelPipeline;
 import io.netty.channel.ChannelPromise;
 import io.netty.channel.socket.SocketChannel;
 import io.netty.handler.codec.ByteToMessageDecoder;
 import io.netty.handler.codec.MessageToByteEncoder;
-import io.netty.util.ReferenceCountUtil;
 import io.netty.util.concurrent.GenericFutureListener;
 import io.netty.util.internal.TypeParameterMatcher;
 
 import java.lang.reflect.InvocationTargetException;
 import java.net.Socket;
 import java.net.SocketAddress;
-import java.nio.channels.ClosedChannelException;
 import java.util.ArrayDeque;
 import java.util.Deque;
 import java.util.List;
@@ -283,37 +280,10 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
 				}
 			};
 
-			ChannelHandlerAdapter exceptionHandler = new ChannelHandlerAdapter() {
-				@Override
-				public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
-					if (channelListener.isDebug()) {
-						// People were complaining about this on the forums, figure I might as well figure out the cause
-						System.out.println("------------ ProtocolLib Debug ------------");
-						System.out.println("Caught an exception in " + playerName + "\'s channel pipeline.");
-						System.out.println("Context: " + ctx);
-						System.out.println("The exception was: " + cause);
-						System.out.println("Stack trace:");
-						cause.printStackTrace(System.out);
-						System.out.println("Please create an issue on GitHub with the above message.");
-						System.out.println("https://github.com/dmulloy2/ProtocolLib/issues");
-						System.out.println("-------------------------------------------");
-					}
-
-					if (cause instanceof ClosedChannelException) {
-						// This is what the DefaultChannelPipeline does
-						ReferenceCountUtil.release(cause);
-					} else {
-						// We only care about closed channel exceptions, pass everything else along
-						super.exceptionCaught(ctx, cause);
-					}
-				}
-			};
-
 			// Insert our handlers - note that we effectively replace the vanilla encoder/decoder
 			originalChannel.pipeline().addBefore("decoder", "protocol_lib_decoder", this);
 			originalChannel.pipeline().addBefore("protocol_lib_decoder", "protocol_lib_finish", finishHandler);
 			originalChannel.pipeline().addAfter("encoder", "protocol_lib_encoder", protocolEncoder);
-			originalChannel.pipeline().addLast("protocol_lib_exception_handler", exceptionHandler);
 
 			// Intercept all write methods
 			channelField.setValue(new NettyChannelProxy(originalChannel, MinecraftReflection.getPacketClass()) {
@@ -840,7 +810,7 @@ public void close() {
 					@Override
 					public void run() {
 						String[] handlers = new String[] {
-								"protocol_lib_decoder", "protocol_lib_finish", "protocol_lib_encoder", "protocol_lib_exception_handler"
+								"protocol_lib_decoder", "protocol_lib_finish", "protocol_lib_encoder"
 						};
 
 						for (String handler : handlers) {

ProtocolLib/src/main/java/com/comphenix/protocol/compat/netty/independent/NettyProtocolInjector.java

@@ -356,7 +356,7 @@ public boolean uninjectPlayer(InetSocketAddress address) {
 
 			@Override
 			public void addPacketHandler(PacketType type, Set<ListenerOptions> options) {
-				if (options != null && !options.contains(ListenerOptions.ASYNC))
+				if (options != null && !type.forceAsync() && !options.contains(ListenerOptions.ASYNC))
 					mainThreadFilters.addType(type);
 				super.addPacketHandler(type, options);
 			}

modules/v1_7_R4/src/main/java/com/comphenix/protocol/compat/netty/shaded/ShadedChannelInjector.java

@@ -19,7 +19,6 @@
 import java.lang.reflect.InvocationTargetException;
 import java.net.Socket;
 import java.net.SocketAddress;
-import java.nio.channels.ClosedChannelException;
 import java.util.ArrayDeque;
 import java.util.Deque;
 import java.util.List;
@@ -32,15 +31,13 @@
 import net.minecraft.util.io.netty.buffer.ByteBuf;
 import net.minecraft.util.io.netty.channel.Channel;
 import net.minecraft.util.io.netty.channel.ChannelHandler;
-import net.minecraft.util.io.netty.channel.ChannelHandlerAdapter;
 import net.minecraft.util.io.netty.channel.ChannelHandlerContext;
 import net.minecraft.util.io.netty.channel.ChannelInboundHandlerAdapter;
 import net.minecraft.util.io.netty.channel.ChannelPipeline;
 import net.minecraft.util.io.netty.channel.ChannelPromise;
 import net.minecraft.util.io.netty.channel.socket.SocketChannel;
 import net.minecraft.util.io.netty.handler.codec.ByteToMessageDecoder;
 import net.minecraft.util.io.netty.handler.codec.MessageToByteEncoder;
-import net.minecraft.util.io.netty.util.ReferenceCountUtil;
 import net.minecraft.util.io.netty.util.concurrent.GenericFutureListener;
 import net.minecraft.util.io.netty.util.internal.TypeParameterMatcher;
 import net.sf.cglib.proxy.Factory;
@@ -282,37 +279,10 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
 				}
 			};
 
-			ChannelHandlerAdapter exceptionHandler = new ChannelHandlerAdapter() {
-				@Override
-				public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
-					if (channelListener.isDebug()) {
-						// People were complaining about this on the forums, figure I might as well figure out the cause
-						System.out.println("------------ ProtocolLib Debug ------------");
-						System.out.println("Caught an exception in " + playerName + "\'s channel pipeline.");
-						System.out.println("Context: " + ctx);
-						System.out.println("The exception was: " + cause);
-						System.out.println("Stack trace:");
-						cause.printStackTrace(System.out);
-						System.out.println("Please create an issue on GitHub with the above message.");
-						System.out.println("https://github.com/dmulloy2/ProtocolLib/issues");
-						System.out.println("-------------------------------------------");
-					}
-
-					if (cause instanceof ClosedChannelException) {
-						// This is what the DefaultChannelPipeline does
-						ReferenceCountUtil.release(cause);
-					} else {
-						// We only care about closed channel exceptions, pass everything else along
-						super.exceptionCaught(ctx, cause);
-					}
-				}
-			};
-
 			// Insert our handlers - note that we effectively replace the vanilla encoder/decoder
 			originalChannel.pipeline().addBefore("decoder", "protocol_lib_decoder", this);
 			originalChannel.pipeline().addBefore("protocol_lib_decoder", "protocol_lib_finish", finishHandler);
 			originalChannel.pipeline().addAfter("encoder", "protocol_lib_encoder", protocolEncoder);
-			originalChannel.pipeline().addLast("protocol_lib_exception_handler", exceptionHandler);
 
 			// Intercept all write methods
 			channelField.setValue(new ShadedChannelProxy(originalChannel, MinecraftReflection.getPacketClass()) {
@@ -839,7 +809,7 @@ public void close() {
 					@Override
 					public void run() {
 						String[] handlers = new String[] {
-								"protocol_lib_decoder", "protocol_lib_finish", "protocol_lib_encoder", "protocol_lib_exception_handler"
+								"protocol_lib_decoder", "protocol_lib_finish", "protocol_lib_encoder"
 						};
 
 						for (String handler : handlers) {

modules/v1_7_R4/src/main/java/com/comphenix/protocol/compat/netty/shaded/ShadedProtocolInjector.java

@@ -356,7 +356,7 @@ public boolean uninjectPlayer(InetSocketAddress address) {
 
 			@Override
 			public void addPacketHandler(PacketType type, Set<ListenerOptions> options) {
-				if (options != null && !options.contains(ListenerOptions.ASYNC))
+				if (options != null && !type.forceAsync() && !options.contains(ListenerOptions.ASYNC))
 					mainThreadFilters.addType(type);
 				super.addPacketHandler(type, options);
 			}