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
This commit is contained in:
Dan Mulloy 2015-10-16 22:36:01 -04:00
parent 9433ea5e48
commit 245433b29a
5 changed files with 23 additions and 82 deletions

View File

@ -269,7 +269,7 @@ public class PacketType implements Serializable, Comparable<PacketType> {
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 class PacketType implements Serializable, Comparable<PacketType> {
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 class PacketType implements Serializable, Comparable<PacketType> {
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 class PacketType implements Serializable, Comparable<PacketType> {
* @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 class PacketType implements Serializable, Comparable<PacketType> {
}
/**
* 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);

View File

@ -19,7 +19,6 @@ package com.comphenix.protocol.compat.netty.independent;
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;
@ -27,14 +26,12 @@ 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 class NettyChannelInjector extends ByteToMessageDecoder implements Channe
}
};
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 class NettyChannelInjector extends ByteToMessageDecoder implements Channe
@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) {

View File

@ -356,7 +356,7 @@ public class NettyProtocolInjector implements ProtocolInjector {
@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);
}

View File

@ -19,7 +19,6 @@ package com.comphenix.protocol.compat.netty.shaded;
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,7 +31,6 @@ import java.util.concurrent.ConcurrentMap;
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;
@ -40,7 +38,6 @@ 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 class ShadedChannelInjector extends ByteToMessageDecoder implements Chann
}
};
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 class ShadedChannelInjector extends ByteToMessageDecoder implements Chann
@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) {

View File

@ -356,7 +356,7 @@ public class ShadedProtocolInjector implements ProtocolInjector {
@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);
}