fix(#3024): cache noop accessor on ctor lookup failure

dmulloy2 · Jun 25, 2024 · e0ea8cf · e0ea8cf
1 parent 32a982e
commit e0ea8cf
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 10 deletions.
diff --git a/src/main/java/com/comphenix/protocol/reflect/accessors/ConstructorAccessor.java b/src/main/java/com/comphenix/protocol/reflect/accessors/ConstructorAccessor.java
@@ -4,6 +4,21 @@
 
 public interface ConstructorAccessor {
 
+    /**
+     * NoOp Accessor, does what is says: nothing.
+     */
+    static final ConstructorAccessor NO_OP_ACCESSOR = new ConstructorAccessor() {
+        @Override
+        public Object invoke(Object... args) {
+            return null;
+        }
+
+        @Override
+        public Constructor<?> getConstructor() {
+            return null;
+        }
+    };
+
     /**
      * Invoke the underlying constructor.
      *

diff --git a/src/main/java/com/comphenix/protocol/reflect/instances/MinecraftGenerator.java b/src/main/java/com/comphenix/protocol/reflect/instances/MinecraftGenerator.java
@@ -1,17 +1,19 @@
 package com.comphenix.protocol.reflect.instances;
 
-import com.comphenix.protocol.reflect.accessors.Accessors;
-import com.comphenix.protocol.reflect.accessors.ConstructorAccessor;
-import com.comphenix.protocol.reflect.accessors.MethodAccessor;
-import com.comphenix.protocol.utility.MinecraftReflection;
-import com.comphenix.protocol.wrappers.BukkitConverters;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.UUID;
+
 import org.bukkit.Material;
 import org.bukkit.entity.EntityType;
 import org.bukkit.inventory.ItemStack;
 
+import com.comphenix.protocol.reflect.accessors.Accessors;
+import com.comphenix.protocol.reflect.accessors.ConstructorAccessor;
+import com.comphenix.protocol.reflect.accessors.MethodAccessor;
+import com.comphenix.protocol.utility.MinecraftReflection;
+import com.comphenix.protocol.wrappers.BukkitConverters;
+
 public class MinecraftGenerator {
 
     // system unique id representation
@@ -58,14 +60,20 @@ public class MinecraftGenerator {
                     try {
                         String name = type.getName();
                         if (name.contains("it.unimi.dsi.fastutil")) {
-                            Class<?> clz = Class.forName(name.replace("Map", "OpenHashMap"));
-                            return Accessors.getConstructorAccessorOrNull(clz);
+                            // convert interface maps to OpenHashMaps
+                            if (!name.endsWith("OpenHashMap")) {
+                                name = name.replace("Map", "OpenHashMap");
+                            }
+
+                            Class<?> hashMapClass = Class.forName(name);
+                            return Accessors.getConstructorAccessorOrNull(hashMapClass);
                         }
                     } catch (Exception ignored) {
                     }
-                    return null;
+                    return ConstructorAccessor.NO_OP_ACCESSOR;
                 });
-                if (ctor != null) {
+
+                if (ctor != ConstructorAccessor.NO_OP_ACCESSOR) {
                     try {
                         return ctor.invoke();
                     } catch (Exception ignored) {

diff --git a/src/test/java/com/comphenix/protocol/injector/netty/channel/PacketListenerInvokerTest.java b/src/test/java/com/comphenix/protocol/injector/netty/channel/PacketListenerInvokerTest.java
@@ -15,7 +15,7 @@ public static void beforeClass() {
     }
 
     @Test
-    public void test() {
+    public void testInitialization() {
         assertDoesNotThrow(() -> PacketListenerInvoker.ensureStaticInitializedWithoutError());
     }
 }
diff --git a/src/test/java/com/comphenix/protocol/reflect/instances/MinecraftGeneratorTest.java b/src/test/java/com/comphenix/protocol/reflect/instances/MinecraftGeneratorTest.java
@@ -0,0 +1,36 @@
+package com.comphenix.protocol.reflect.instances;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.util.UUID;
+
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import com.comphenix.protocol.BukkitInitialization;
+import com.comphenix.protocol.PacketType;
+
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
+import net.minecraft.core.NonNullList;
+import net.minecraft.world.entity.EntityTypes;
+import net.minecraft.world.item.ItemStack;
+
+public class MinecraftGeneratorTest {
+
+    @BeforeAll
+    public static void beforeClass() {
+        BukkitInitialization.initializeAll();
+    }
+
+    @Test
+    public void testInstantiation() {
+        assertNotNull(MinecraftGenerator.INSTANCE.create(UUID.class));
+        assertNotNull(MinecraftGenerator.INSTANCE.create(PacketType.Protocol.class));
+        assertNotNull(MinecraftGenerator.INSTANCE.create(ItemStack.class));
+        assertNotNull(MinecraftGenerator.INSTANCE.create(EntityTypes.class));
+        assertNotNull(MinecraftGenerator.INSTANCE.create(Int2ObjectMap.class));
+        assertNotNull(MinecraftGenerator.INSTANCE.create(Int2ObjectOpenHashMap.class));
+        assertNotNull(MinecraftGenerator.INSTANCE.create(NonNullList.class));   
+    }
+}