From 90548f1092d8c42f470fe2ba387869ee21c5ccad Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Fri, 17 Jan 2014 02:51:22 +0100 Subject: [PATCH] Periodically clean up injectors in Spigot 1.6.4 and earlier. This (may) fix a fairly serious memory leak. --- .../injector/PacketFilterManager.java | 6 +- .../injector/spigot/SpigotPacketInjector.java | 64 +++++++++++++++++-- 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java index dc5215af..991dda50 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java @@ -98,6 +98,11 @@ public final class PacketFilterManager implements ProtocolManager, ListenerInvok public static final ReportType REPORT_CANNOT_UNREGISTER_PLUGIN = new ReportType("Unable to handle disabled plugin."); public static final ReportType REPORT_PLUGIN_VERIFIER_ERROR = new ReportType("Verifier error: %s"); + /** + * The number of ticks in a second. + */ + public static final int TICKS_PER_SECOND = 20; + /** * Sets the inject hook type. Different types allow for maximum compatibility. * @author Kristian @@ -129,7 +134,6 @@ public final class PacketFilterManager implements ProtocolManager, ListenerInvok } // The amount of time to wait until we actually unhook every player - private static final int TICKS_PER_SECOND = 20; private static final int UNHOOK_DELAY = 5 * TICKS_PER_SECOND; // Delayed unhook diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/spigot/SpigotPacketInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/spigot/SpigotPacketInjector.java index a2e3d165..b782e1d2 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/spigot/SpigotPacketInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/spigot/SpigotPacketInjector.java @@ -24,6 +24,7 @@ import net.sf.cglib.proxy.NoOp; import com.comphenix.protocol.PacketType; import com.comphenix.protocol.PacketType.Sender; +import com.comphenix.protocol.ProtocolLibrary; import com.comphenix.protocol.concurrency.PacketTypeSet; import com.comphenix.protocol.error.DelegatedErrorReporter; import com.comphenix.protocol.error.ErrorReporter; @@ -34,6 +35,8 @@ import com.comphenix.protocol.events.NetworkMarker; import com.comphenix.protocol.events.PacketContainer; import com.comphenix.protocol.events.PacketEvent; import com.comphenix.protocol.injector.ListenerInvoker; +import com.comphenix.protocol.injector.PacketFilterBuilder; +import com.comphenix.protocol.injector.PacketFilterManager; import com.comphenix.protocol.injector.PlayerLoggedOutException; import com.comphenix.protocol.injector.packet.LegacyNetworkMarker; import com.comphenix.protocol.injector.packet.PacketInjector; @@ -103,6 +106,10 @@ public class SpigotPacketInjector implements SpigotPacketListener { // The proxy packet injector private PacketInjector proxyPacketInjector; + // Background task + private static final int BACKGROUND_DELAY = 30 * PacketFilterManager.TICKS_PER_SECOND; + private int backgroundId; + /** * Create a new spigot injector. */ @@ -113,7 +120,7 @@ public class SpigotPacketInjector implements SpigotPacketListener { this.queuedFilters = new PacketTypeSet(); this.reveivedFilters = new PacketTypeSet(); } - + /** * Retrieve the underlying listener invoker. * @return The invoker. @@ -253,10 +260,55 @@ public class SpigotPacketInjector implements SpigotPacketListener { throw new RuntimeException("Cannot register Spigot packet listener.", e); } + // Remember to register background task + backgroundId = createBackgroundTask(); + // If we succeed return true; } + /** + * Create and register a background task. + * @return The background task ID. + */ + private int createBackgroundTask() { + return Bukkit.getScheduler().scheduleSyncRepeatingTask(plugin, new Runnable() { + @Override + public void run() { + cleanupInjectors(); + } + }, BACKGROUND_DELAY, BACKGROUND_DELAY); + } + + /** + * Ensure that all disconnected injectors are removed from memory. + */ + private void cleanupInjectors() { + for (NetworkObjectInjector injector : networkManagerInjector.values()) { + try { + if (injector.getSocket() != null && injector.getSocket().isClosed()) { + cleanupInjector(injector); + } + } catch (Exception e) { + reporter.reportMinimal(plugin, "cleanupInjectors", e); + + // What? + cleanupInjector(injector); + } + } + } + + /** + * Remove a given network object injector. + * @param injector - the injector. + */ + private void cleanupInjector(final NetworkObjectInjector injector) { + // Clean up + playerInjector.remove(injector.getPlayer()); + playerInjector.remove(injector.getUpdatedPlayer()); + networkManagerInjector.remove(injector.getNetworkManager()); + } + /** * Determine if the given method is a valid packet receiver or queued method. * @param methodName - the expected name of the method. @@ -530,14 +582,12 @@ public class SpigotPacketInjector implements SpigotPacketListener { Bukkit.getScheduler().scheduleSyncDelayedTask(plugin, new Runnable() { @Override public void run() { - // Clean up - playerInjector.remove(injector.getPlayer()); - playerInjector.remove(injector.getUpdatedPlayer()); - networkManagerInjector.remove(injector.getNetworkManager()); + cleanupInjector(injector); } }, CLEANUP_DELAY); } } + /** * Invoked when a plugin wants to sent a packet. * @param receiver - the packet receiver. @@ -609,6 +659,10 @@ public class SpigotPacketInjector implements SpigotPacketListener { if (dynamicListener != null) { cleanupListener(); } + if (backgroundId >= 0) { + Bukkit.getScheduler().cancelTask(backgroundId); + backgroundId = -1; + } // Cleanup network marker if (proxyPacketInjector != null) {