Merge remote-tracking branch 'origin/feature/weak-references' into development

core/src/main/java/org/geysermc/floodgate/inject/CommonPlatformInjector.java

@@ -26,18 +26,17 @@
 package org.geysermc.floodgate.inject;
 
 import io.netty.channel.Channel;
+import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
-import lombok.AccessLevel;
-import lombok.Getter;
+import java.util.WeakHashMap;
 import org.geysermc.floodgate.api.inject.InjectorAddon;
 import org.geysermc.floodgate.api.inject.PlatformInjector;
 
 public abstract class CommonPlatformInjector implements PlatformInjector {
-    @Getter(AccessLevel.PROTECTED)
-    private final Set<Channel> injectedClients = new HashSet<>();
+    private final Set<Channel> injectedClients =
+            Collections.synchronizedSet(Collections.newSetFromMap(new WeakHashMap<>()));
 
     private final Map<Class<?>, InjectorAddon> addons = new HashMap<>();
 
@@ -49,6 +48,10 @@ public abstract class CommonPlatformInjector implements PlatformInjector {
         return injectedClients.remove(channel);
     }
 
+    public Set<Channel> injectedClients() {
+        return injectedClients;
+    }
+
     @Override
     public boolean addAddon(InjectorAddon addon) {
         return addons.putIfAbsent(addon.getClass(), addon) == null;

spigot/src/main/java/org/geysermc/floodgate/inject/spigot/SpigotInjector.java

@@ -25,6 +25,8 @@
 
 package org.geysermc.floodgate.inject.spigot;
 
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelFuture;
 import io.netty.channel.ChannelHandlerContext;
@@ -36,13 +38,15 @@ import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.util.List;
 import lombok.Getter;
-import lombok.RequiredArgsConstructor;
+import org.geysermc.floodgate.api.logger.FloodgateLogger;
 import org.geysermc.floodgate.inject.CommonPlatformInjector;
 import org.geysermc.floodgate.util.ClassNames;
 import org.geysermc.floodgate.util.ReflectionUtils;
 
-@RequiredArgsConstructor
+@Singleton
 public final class SpigotInjector extends CommonPlatformInjector {
+    @Inject private FloodgateLogger logger;
+
     private Object serverConnection;
     private String injectedFieldName;
 
@@ -126,30 +130,43 @@ public final class SpigotInjector extends CommonPlatformInjector {
             return;
         }
 
-        // remove injection from clients
-        for (Channel channel : getInjectedClients()) {
-            removeAddonsCall(channel);
-        }
-        getInjectedClients().clear();
-
-        // and change the list back to the original
+        // let's change the list back to the original first
+        // so that new connections are not handled through our custom list
         Object serverConnection = getServerConnection();
         if (serverConnection != null) {
             Field field = ReflectionUtils.getField(serverConnection.getClass(), injectedFieldName);
-            List<?> list = (List<?>) ReflectionUtils.getValue(serverConnection, field);
+            Object value = ReflectionUtils.getValue(serverConnection, field);
 
-            if (list instanceof CustomList) {
-                CustomList customList = (CustomList) list;
+            if (value instanceof CustomList) {
+                // all we have to do is replace the list with the original list.
+                // the original list is up-to-date, so we don't have to clear/add/whatever anything
+                CustomList customList = (CustomList) value;
                 ReflectionUtils.setValue(serverConnection, field, customList.getOriginalList());
-                customList.clear();
-                customList.addAll(list);
+                return;
             }
+
+            // we could replace all references of CustomList that are directly in 'value', but that
+            // only brings you so far. ProtocolLib for example stores the original value
+            // (which would be our CustomList e.g.) in a separate object
+            logger.debug(
+                    "Unable to remove all references of Floodgate due to {}! ",
+                    value.getClass().getName()
+            );
         }
 
+        // remove injection from clients
+        for (Channel channel : injectedClients()) {
+            removeAddonsCall(channel);
+        }
+
+        //todo make sure that all references are removed from the channels,
+        // except from one AttributeKey with Floodgate player data which could be used
+        // after reloading
+
         injected = false;
     }
 
-    public Object getServerConnection() {
+    private Object getServerConnection() {
         if (serverConnection != null) {
             return serverConnection;
         }

spigot/src/main/java/org/geysermc/floodgate/module/SpigotPlatformModule.java

@@ -63,6 +63,7 @@ public final class SpigotPlatformModule extends AbstractModule {
     protected void configure() {
         bind(SpigotPlugin.class).toInstance(plugin);
         bind(PlatformUtils.class).to(SpigotPlatformUtils.class);
+        bind(CommonPlatformInjector.class).to(SpigotInjector.class);
         bind(Logger.class).annotatedWith(Names.named("logger")).toInstance(plugin.getLogger());
         bind(FloodgateLogger.class).to(JavaUtilFloodgateLogger.class);
         bind(SkinApplier.class).to(SpigotSkinApplier.class);
@@ -98,12 +99,6 @@ public final class SpigotPlatformModule extends AbstractModule {
     DebugAddon / PlatformInjector
      */
 
-    @Provides
-    @Singleton
-    public CommonPlatformInjector platformInjector() {
-        return new SpigotInjector();
-    }
-
     @Provides
     @Named("packetEncoder")
     public String packetEncoder() {