Don't attempt to process packets after disconnect

Addresses #28

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

@@ -170,7 +170,7 @@ public class ProtocolConfig {
 			loadingSections = false;
 			
 			// Inform the user
-			System.out.println("[ProtocolLib] Created default configuration.");
+			ProtocolLibrary.log("Created default configuration.");
 		}
 	}
 	

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

@@ -19,6 +19,7 @@ package com.comphenix.protocol;
 
 import java.io.File;
 import java.io.IOException;
+import java.text.MessageFormat;
 import java.util.Set;
 import java.util.logging.Handler;
 import java.util.logging.Level;
@@ -140,7 +141,7 @@ public class ProtocolLibrary extends JavaPlugin {
 	private int configExpectedMod = -1;
 
 	// Logger
-	private Logger logger;
+	private static Logger logger;
 	private Handler redirectHandler;
 
 	// Commands
@@ -652,4 +653,14 @@ public class ProtocolLibrary extends JavaPlugin {
 	public static ListeningScheduledExecutorService getExecutorSync() {
 		return executorSync;
 	}
+
+	// ---- Logging
+
+	public static void log(Level level, String message, Object... args) {
+		logger.log(level, MessageFormat.format(message, args));
+	}
+
+	public static void log(String message, Object... args) {
+		log(Level.INFO, message, args);
+	}
 }

ProtocolLib/src/main/java/com/comphenix/protocol/async/AsyncMarker.java

@@ -24,9 +24,11 @@ import java.lang.reflect.Method;
 import java.util.Iterator;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.logging.Level;
 
 import com.comphenix.protocol.PacketStream;
 import com.comphenix.protocol.PacketType;
+import com.comphenix.protocol.ProtocolLibrary;
 import com.comphenix.protocol.events.NetworkMarker;
 import com.comphenix.protocol.events.PacketEvent;
 import com.comphenix.protocol.injector.PrioritizedListener;
@@ -426,7 +428,7 @@ public class AsyncMarker implements Serializable, Comparable<AsyncMarker> {
 							// Incoming chat packets are async only if they aren't commands
 							return ! content.startsWith("/");
 						} else {
-							System.err.println("[ProtocolLib] Failed to determine contents of incoming chat packet!");
+							ProtocolLibrary.log(Level.WARNING, "Failed to determine contents of incoming chat packet!");
 							alwaysSync = true;
 						}
 					} else {
@@ -434,7 +436,7 @@ public class AsyncMarker implements Serializable, Comparable<AsyncMarker> {
 						return false;
 					}
 				} else {
-					System.err.println("[ProtocolLib] Cannot determine asynchronous state of packets!");
+					ProtocolLibrary.log(Level.WARNING, "Cannot determine asynchronous state of packets!");
 					alwaysSync = true;
 				}
 			}

ProtocolLib/src/main/java/com/comphenix/protocol/error/DetailedErrorReporter.java

@@ -35,6 +35,7 @@ import org.apache.commons.lang.builder.ToStringStyle;
 import org.bukkit.Bukkit;
 import org.bukkit.plugin.Plugin;
 
+import com.comphenix.protocol.ProtocolLibrary;
 import com.comphenix.protocol.collections.ExpireHashMap;
 import com.comphenix.protocol.error.Report.ReportBuilder;
 import com.comphenix.protocol.events.PacketAdapter;
@@ -456,7 +457,7 @@ public class DetailedErrorReporter implements ErrorReporter {
 				apacheCommonsMissing = true;
 			} catch (Exception e) {
 				// Don't use the error logger to log errors in error logging (that could lead to infinite loops)
-				System.err.print("[ProtocolLib] Warning: Cannot convert to a String with Apache: " + e.getMessage());
+				ProtocolLibrary.log(Level.WARNING, "Cannot convert to a String with Apache: " + e.getMessage());
 			}
 			
 			// Use our custom object printer instead

ProtocolLib/src/main/java/com/comphenix/protocol/injector/netty/ChannelInjector.java

@@ -14,6 +14,7 @@ import io.netty.handler.codec.MessageToByteEncoder;
 import io.netty.util.concurrent.GenericFutureListener;
 import io.netty.util.internal.TypeParameterMatcher;
 
+import java.lang.ref.WeakReference;
 import java.lang.reflect.InvocationTargetException;
 import java.net.Socket;
 import java.net.SocketAddress;
@@ -84,9 +85,10 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector {
 	// The factory that created this injector
 	private InjectionFactory factory;
 
-	// The player, or temporary player
+	// The player
-	private Player player;
+	private WeakReference<Player> player;
-	private Player updated;
+	private WeakReference<Player> updated;
+	private String playerName;
 
 	// The player connection
 	private Object playerConnection;
@@ -149,7 +151,7 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector {
 	 * @param factory - the factory that created this injector
 	 */
 	public ChannelInjector(Player player, Object networkManager, Channel channel, ChannelListener channelListener, InjectionFactory factory) {
-		this.player =  Preconditions.checkNotNull(player, "player cannot be NULL");
+		this.player =  new WeakReference<Player>(Preconditions.checkNotNull(player, "player cannot be NULL"));
 		this.networkManager =  Preconditions.checkNotNull(networkManager, "networkMananger cannot be NULL");
 		this.originalChannel = Preconditions.checkNotNull(channel, "channel cannot be NULL");
 		this.channelListener = Preconditions.checkNotNull(channelListener, "channelListener cannot be NULL");
@@ -486,6 +488,11 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector {
 			// Reset queue
 			finishQueue.clear();
 
+			if (player.get() == null) {
+				ProtocolLibrary.log("Failed to intercept client packet for {0}: Invalid player.", playerName);
+				return;
+			}
+
 			for (ListIterator<Object> it = packets.listIterator(); it.hasNext(); ) {
 				Object input = it.next();
 				Class<?> packetClass = input.getClass();
@@ -498,6 +505,7 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector {
 					byteBuffer.resetReaderIndex();
 					marker = new NettyNetworkMarker(ConnectionSide.CLIENT_SIDE, getBytes(byteBuffer));
 				}
+
 				PacketEvent output = channelListener.onPacketReceiving(this, input, marker);
 
 				// Handle packet changes
@@ -674,7 +682,7 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector {
 	 */
 	private Object getPlayerConnection() {
 		if (playerConnection == null) {
-			playerConnection = MinecraftFields.getPlayerConnection(player);
+			playerConnection = MinecraftFields.getPlayerConnection(player.get());
 		}
 		return playerConnection;
 	}
@@ -693,7 +701,7 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector {
 
 	@Override
 	public Player getPlayer() {
-		return player;
+		return player.get();
 	}
 
 	/**
@@ -702,7 +710,7 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector {
 	 */
 	@Override
     public void setPlayer(Player player) {
-		this.player = player;
+		this.player = new WeakReference<Player>(Preconditions.checkNotNull(player, "player cannot be null"));
 	}
 
 	/**
@@ -711,7 +719,7 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector {
 	 */
 	@Override
     public void setUpdatedPlayer(Player updated) {
-		this.updated = updated;
+		this.updated = new WeakReference<Player>(Preconditions.checkNotNull(updated, "updated cannot be null"));;
 	}
 
 	@Override
@@ -752,7 +760,7 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector {
 				// we end up with a deadlock. The main thread is waiting for the worker thread to process the task, and
 			    // the worker thread is waiting for the main thread to finish executing PlayerQuitEvent.
 				//
-				// TLDR: Concurrenty is hard.
+				// TLDR: Concurrency is hard.
 				executeInChannelThread(new Runnable() {
 					@Override
 					public void run() {
@@ -767,14 +775,14 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector {
 				});
 
 				// Clear cache
-				factory.invalidate(player);
+				factory.invalidate(player.get());
 			}
 		}
 
-		// dmulloy2 - clear player instances
+		// Clear player references
-		// Should fix memory leaks
+		// Should help fix memory leaks
-		this.player = null;
+		player.clear();
-		this.updated = null;
+		updated.clear();
 	}
 
 	/**
@@ -845,12 +853,12 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector {
 
 		@Override
 		public Player getPlayer() {
-			return injector.player;
+			return injector.player.get();
 		}
 
 		@Override
 		public Player getUpdatedPlayer() {
-			return injector.updated;
+			return injector.updated.get();
 		}
 
 		@Override
@@ -860,7 +868,7 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector {
 
 		@Override
 		public void setUpdatedPlayer(Player updatedPlayer) {
-			injector.player = updatedPlayer;
+			injector.player = new WeakReference<Player>(updatedPlayer);
 		}
 
 		public ChannelInjector getChannelInjector() {

ProtocolLib/src/main/java/com/comphenix/protocol/reflect/VolatileField.java

@@ -18,7 +18,9 @@
 package com.comphenix.protocol.reflect;
 
 import java.lang.reflect.Field;
+import java.util.logging.Level;
 
+import com.comphenix.protocol.ProtocolLibrary;
 import com.comphenix.protocol.reflect.accessors.Accessors;
 import com.comphenix.protocol.reflect.accessors.FieldAccessor;
 import com.google.common.base.Objects;
@@ -183,8 +185,8 @@ public class VolatileField {
 				currentSet = false;
 			} else {
 				// This can be a bad sign
-				System.out.println(String.format("[ProtocolLib] Unable to switch %s to %s. Expected %s but got %s.",
+				ProtocolLibrary.log(Level.WARNING, "Unable to switch %s to %s. Expected %s but got %s.",
-						getField().toGenericString(), previous, current, getValue()));
+						getField().toGenericString(), previous, current, getValue());
 			}
 		}
 	}

ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java

@@ -34,6 +34,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentMap;
+import java.util.logging.Level;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -216,7 +217,7 @@ public class MinecraftReflection {
 					if (MinecraftVersion.SCARY_UPDATE.compareTo(version) <= 0) {
 						 // Just assume R1 - it's probably fine
 						packageVersion = "v" + version.getMajor() + "_" + version.getMinor() + "_R1";
-						System.err.println("[ProtocolLib] Assuming package version: " + packageVersion);
+						ProtocolLibrary.log(Level.WARNING, "Assuming package version: " + packageVersion);
 					}
 				}