From 801bf81f153044b5621c916cb33dc5234173ab80 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Sun, 30 Sep 2012 03:24:13 +0200 Subject: [PATCH] Incredibly difficult bug to track down. Forgot to remove the "override" flag on cancelled packets when they're sent again. --- .../protocol/async/AsyncFilterManager.java | 8 +++--- .../protocol/async/PacketSendingQueue.java | 3 +- .../injector/PacketFilterManager.java | 3 ++ .../protocol/injector/PacketInjector.java | 28 +++++++++++++++++-- .../protocol/injector/ReadPacketModifier.java | 17 +++++++++++ 5 files changed, 51 insertions(+), 8 deletions(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/async/AsyncFilterManager.java b/ProtocolLib/src/com/comphenix/protocol/async/AsyncFilterManager.java index 7d7ee841..a09bfd13 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/AsyncFilterManager.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/AsyncFilterManager.java @@ -74,22 +74,22 @@ public class AsyncFilterManager implements AsynchronousManager { ListeningWhitelist receivingWhitelist = listener.getReceivingWhitelist(); // We need a synchronized listener to get the ball rolling - boolean needNoOp = true; + boolean hasListener = true; // Add listener to either or both processing queue if (hasValidWhitelist(sendingWhitelist)) { PacketFilterManager.verifyWhitelist(listener, sendingWhitelist); serverProcessingQueue.addListener(handler, sendingWhitelist); - needNoOp &= hasPacketListener(sendingWhitelist); + hasListener &= hasPacketListener(sendingWhitelist); } if (hasValidWhitelist(receivingWhitelist)) { PacketFilterManager.verifyWhitelist(listener, receivingWhitelist); clientProcessingQueue.addListener(handler, receivingWhitelist); - needNoOp &= hasPacketListener(receivingWhitelist); + hasListener &= hasPacketListener(receivingWhitelist); } - if (needNoOp) { + if (!hasListener) { handler.setNullPacketListener(new NullPacketListener(listener)); manager.addPacketListener(handler.getNullPacketListener()); } diff --git a/ProtocolLib/src/com/comphenix/protocol/async/PacketSendingQueue.java b/ProtocolLib/src/com/comphenix/protocol/async/PacketSendingQueue.java index be20847c..4d25a667 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/PacketSendingQueue.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/PacketSendingQueue.java @@ -97,8 +97,9 @@ class PacketSendingQueue { AsyncMarker marker = current.getAsyncMarker(); if (marker.isProcessed() || marker.hasExpired()) { - if (marker.isProcessed() && !current.isCancelled()) + if (marker.isProcessed() && !current.isCancelled()) { sendPacket(current); + } sendingQueue.poll(); continue; diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java index 22a5b232..1dfb5bf9 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java @@ -399,6 +399,9 @@ public final class PacketFilterManager implements ProtocolManager { PlayerInjector injector = getInjector(sender); Packet mcPacket = packet.getHandle(); + // Make sure the packet isn't cancelled + packetInjector.undoCancel(packet.getID(), mcPacket); + if (filters) { mcPacket = injector.handlePacketRecieved(mcPacket); } diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PacketInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/PacketInjector.java index ebdfd578..998eb5a7 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PacketInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PacketInjector.java @@ -23,6 +23,7 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import org.bukkit.entity.Player; @@ -52,6 +53,9 @@ class PacketInjector { // Allows us to determine the sender private Map playerLookup; + // Allows us to look up read packet injectors + private Map readModifier; + // Class loader private ClassLoader classLoader; @@ -61,9 +65,24 @@ class PacketInjector { this.classLoader = classLoader; this.manager = manager; this.playerLookup = playerLookup; + this.readModifier = new ConcurrentHashMap(); initialize(); } + /** + * Undo a packet cancel. + * @param id - the id of the packet. + * @param packet - packet to uncancel. + */ + public void undoCancel(Integer id, Packet packet) { + ReadPacketModifier modifier = readModifier.get(id); + + // Cancelled packets are represented with NULL + if (modifier != null && modifier.getOverride(packet) == null) { + modifier.removeOverride(packet); + } + } + private void initialize() throws IllegalAccessException { if (intHashMap == null) { // We're looking for the first static field with a Minecraft-object. This should be a IntHashMap. @@ -109,10 +128,12 @@ class PacketInjector { ex.setClassLoader(classLoader); Class proxy = ex.createClass(); + // Create the proxy handler + ReadPacketModifier modifier = new ReadPacketModifier(packetID, this); + readModifier.put(packetID, modifier); + // Add a static reference - Enhancer.registerStaticCallbacks(proxy, new Callback[] { - new ReadPacketModifier(packetID, this) - }); + Enhancer.registerStaticCallbacks(proxy, new Callback[] { modifier }); try { // Override values @@ -147,6 +168,7 @@ class PacketInjector { putMethod.invoke(intHashMap, packetID, old); previous.remove(packetID); + readModifier.remove(packetID); registry.remove(proxy); overwritten.remove(packetID); return true; diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/ReadPacketModifier.java b/ProtocolLib/src/com/comphenix/protocol/injector/ReadPacketModifier.java index d8400125..ba489b96 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/ReadPacketModifier.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/ReadPacketModifier.java @@ -45,6 +45,23 @@ class ReadPacketModifier implements MethodInterceptor { this.packetID = packetID; this.packetInjector = packetInjector; } + + /** + * Remove any packet overrides. + * @param packet - the packet to rever + */ + public void removeOverride(Packet packet) { + override.remove(packet); + } + + /** + * Retrieve the packet that overrides the methods of the given packet. + * @param packet - the given packet. + * @return Overriden object. + */ + public Object getOverride(Packet packet) { + return override.get(packet); + } @Override public Object intercept(Object thisObj, Method method, Object[] args, MethodProxy proxy) throws Throwable {