From a1b989cb3a01f83ff605c5a18541d549979b6e4c Mon Sep 17 00:00:00 2001 From: Dan Mulloy Date: Thu, 1 Jan 2015 21:06:11 -0500 Subject: [PATCH] Revert changes to channel injection --- .../comphenix/protocol/ProtocolLibrary.java | 2 +- .../injector/netty/ChannelInjector.java | 46 ++++++++----------- .../protocol/reflect/VolatileField.java | 3 +- .../reflect/instances/DefaultInstances.java | 6 +-- .../protocol/wrappers/WrappedDataWatcher.java | 2 +- 5 files changed, 25 insertions(+), 34 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java b/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java index 089b36cb..3ef0414f 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java @@ -654,7 +654,7 @@ public class ProtocolLibrary extends JavaPlugin { return executorSync; } - // ---- Logging + // ---- Logging Methods public static void log(Level level, String message, Object... args) { logger.log(level, MessageFormat.format(message, args)); diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/netty/ChannelInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/netty/ChannelInjector.java index 28dea810..93e680aa 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/netty/ChannelInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/netty/ChannelInjector.java @@ -14,7 +14,6 @@ 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; @@ -85,10 +84,9 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector { // The factory that created this injector private InjectionFactory factory; - // The player - private WeakReference player; - private WeakReference updated; - private String playerName; + // The player, or temporary player + private Player player; + private Player updated; // The player connection private Object playerConnection; @@ -151,7 +149,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 = new WeakReference(Preconditions.checkNotNull(player, "player cannot be NULL")); + this.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"); @@ -488,11 +486,6 @@ 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 it = packets.listIterator(); it.hasNext(); ) { Object input = it.next(); Class packetClass = input.getClass(); @@ -505,7 +498,6 @@ 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 @@ -600,7 +592,7 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector { */ private void disconnect(String message) { // If we're logging in, we can only close the channel - if (playerConnection == null || player.get() instanceof Factory) { + if (playerConnection == null || player instanceof Factory) { originalChannel.disconnect(); } else { // Call the disconnect method @@ -632,7 +624,7 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector { private void invokeSendPacket(Object packet) { // Attempt to send the packet with NetworkMarker.handle(), or the PlayerConnection if its active try { - if (player.get() instanceof Factory) { + if (player instanceof Factory) { MinecraftMethods.getNetworkManagerHandleMethod().invoke(networkManager, packet, new GenericFutureListener[0]); } else { MinecraftMethods.getSendPacketMethod().invoke(getPlayerConnection(), packet); @@ -682,7 +674,7 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector { */ private Object getPlayerConnection() { if (playerConnection == null) { - playerConnection = MinecraftFields.getPlayerConnection(player.get()); + playerConnection = MinecraftFields.getPlayerConnection(player); } return playerConnection; } @@ -701,7 +693,7 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector { @Override public Player getPlayer() { - return player.get(); + return player; } /** @@ -710,7 +702,7 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector { */ @Override public void setPlayer(Player player) { - this.player = new WeakReference(Preconditions.checkNotNull(player, "player cannot be null")); + this.player = player; } /** @@ -719,7 +711,7 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector { */ @Override public void setUpdatedPlayer(Player updated) { - this.updated = new WeakReference(Preconditions.checkNotNull(updated, "updated cannot be null"));; + this.updated = updated; } @Override @@ -760,7 +752,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: Concurrency is hard. + // TLDR: Concurrenty is hard. executeInChannelThread(new Runnable() { @Override public void run() { @@ -775,14 +767,14 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector { }); // Clear cache - factory.invalidate(player.get()); + factory.invalidate(player); } } - // Clear player references - // Should help fix memory leaks - player.clear(); - updated.clear(); + // dmulloy2 - clear player instances + // Should fix memory leaks + this.player = null; + this.updated = null; } /** @@ -853,12 +845,12 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector { @Override public Player getPlayer() { - return injector.player.get(); + return injector.player; } @Override public Player getUpdatedPlayer() { - return injector.updated.get(); + return injector.updated; } @Override @@ -868,7 +860,7 @@ class ChannelInjector extends ByteToMessageDecoder implements Injector { @Override public void setUpdatedPlayer(Player updatedPlayer) { - injector.player = new WeakReference(updatedPlayer); + injector.player = updatedPlayer; } public ChannelInjector getChannelInjector() { diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/VolatileField.java b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/VolatileField.java index b78dc520..c4318c60 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/VolatileField.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/VolatileField.java @@ -18,7 +18,6 @@ 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; @@ -185,7 +184,7 @@ public class VolatileField { currentSet = false; } else { // This can be a bad sign - ProtocolLibrary.log(Level.WARNING, "Unable to switch %s to %s. Expected %s but got %s.", + ProtocolLibrary.log("Unable to switch {0} to {1}. Expected {2}, but got {3}.", getField().toGenericString(), previous, current, getValue()); } } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/instances/DefaultInstances.java b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/instances/DefaultInstances.java index 55f9afde..3e13de80 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/instances/DefaultInstances.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/instances/DefaultInstances.java @@ -20,11 +20,13 @@ package com.comphenix.protocol.reflect.instances; import java.lang.reflect.Constructor; import java.util.Collection; import java.util.List; +import java.util.logging.Level; import javax.annotation.Nullable; import net.sf.cglib.proxy.Enhancer; +import com.comphenix.protocol.ProtocolLibrary; import com.google.common.base.Objects; import com.google.common.collect.ImmutableList; @@ -281,7 +283,7 @@ public class DefaultInstances implements InstanceProvider { // Did we break the non-null contract? if (params[i] == null && nonNull) { - System.out.println("Nonnull contract broken."); + ProtocolLibrary.log(Level.WARNING, "Nonnull contract broken."); return null; } } @@ -328,8 +330,6 @@ public class DefaultInstances implements InstanceProvider { try { return constructor.newInstance(params); } catch (Exception e) { - // System.out.println("Failed to create instance " + constructor); - // e.printStackTrace(); return null; } } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedDataWatcher.java b/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedDataWatcher.java index bd59d58d..43e2c169 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedDataWatcher.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedDataWatcher.java @@ -79,7 +79,7 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable 1 ? Accessors.getFieldAccessor(spigotClass, "value2", true) : null; // // } catch (ClassNotFoundException e) { -// System.out.println("[ProtocolLib] Unable to find " + className); +// ProtocolLibrary.log(Level.WARNING, "Unable to find " + className); // this.spigotClass = null; // } // this.typeId = typeId;