diff --git a/src/main/java/net/minestom/server/event/EventNodeImpl.java b/src/main/java/net/minestom/server/event/EventNodeImpl.java index c364b824d..0a1706111 100644 --- a/src/main/java/net/minestom/server/event/EventNodeImpl.java +++ b/src/main/java/net/minestom/server/event/EventNodeImpl.java @@ -1,5 +1,6 @@ package net.minestom.server.event; +import com.github.benmanes.caffeine.cache.Caffeine; import net.minestom.server.MinecraftServer; import net.minestom.server.event.trait.RecursiveEvent; import net.minestom.server.utils.validate.Check; @@ -20,9 +21,12 @@ non-sealed class EventNodeImpl implements EventNode { private final Map> handleMap = new ConcurrentHashMap<>(); final Map, ListenerEntry> listenerMap = new ConcurrentHashMap<>(); - final Set> children = new CopyOnWriteArraySet<>(); - final Map> mappedNodeCache = new WeakHashMap<>(); - final Map> registeredMappedNode = new WeakHashMap<>(); + final Set> children = Collections.newSetFromMap(Caffeine.newBuilder() + .weakKeys()., Boolean>build().asMap()); + final Map> mappedNodeCache = Caffeine.newBuilder() + .weakKeys().weakValues().>build().asMap(); + final Map> registeredMappedNode = Caffeine.newBuilder() + .weakKeys().weakValues().>build().asMap(); final String name; final EventFilter filter; @@ -50,6 +54,7 @@ non-sealed class EventNodeImpl implements EventNode { @Override public @NotNull List> findChildren(@NotNull String name, Class eventType) { synchronized (GLOBAL_CHILD_LOCK) { + final Set> children = getChildren(); if (children.isEmpty()) return List.of(); List> result = new ArrayList<>(); for (EventNode child : children) { @@ -70,6 +75,7 @@ non-sealed class EventNodeImpl implements EventNode { @Override public void replaceChildren(@NotNull String name, @NotNull Class eventType, @NotNull EventNode eventNode) { synchronized (GLOBAL_CHILD_LOCK) { + final Set> children = getChildren(); if (children.isEmpty()) return; for (EventNode child : children) { if (equals(child, name, eventType)) { @@ -85,6 +91,7 @@ non-sealed class EventNodeImpl implements EventNode { @Override public void removeChildren(@NotNull String name, @NotNull Class eventType) { synchronized (GLOBAL_CHILD_LOCK) { + final Set> children = getChildren(); if (children.isEmpty()) return; for (EventNode child : children) { if (equals(child, name, eventType)) { diff --git a/src/test/java/net/minestom/server/event/EventNodeQueryTest.java b/src/test/java/net/minestom/server/event/EventNodeQueryTest.java index 7d519dda4..9e035b0f7 100644 --- a/src/test/java/net/minestom/server/event/EventNodeQueryTest.java +++ b/src/test/java/net/minestom/server/event/EventNodeQueryTest.java @@ -6,6 +6,7 @@ import org.junit.jupiter.api.Test; import java.util.List; +import static net.minestom.testing.TestUtils.assertEqualsIgnoreOrder; import static org.junit.jupiter.api.Assertions.assertEquals; public class EventNodeQueryTest { @@ -23,12 +24,12 @@ public class EventNodeQueryTest { node.addChild(child2); node.addChild(child3); - assertEquals(List.of(child1, child2), node.findChildren("test")); - assertEquals(List.of(child3), node.findChildren("test3")); + assertEqualsIgnoreOrder(List.of(child1, child2), node.findChildren("test")); + assertEqualsIgnoreOrder(List.of(child3), node.findChildren("test3")); node.removeChild(child1); - assertEquals(List.of(child2), node.findChildren("test")); - assertEquals(List.of(child3), node.findChildren("test3")); + assertEqualsIgnoreOrder(List.of(child2), node.findChildren("test")); + assertEqualsIgnoreOrder(List.of(child3), node.findChildren("test3")); } @Test @@ -44,16 +45,16 @@ public class EventNodeQueryTest { node.addChild(child2); node.addChild(child3); - assertEquals(List.of(child1, child2), node.findChildren("test", Event.class)); - assertEquals(List.of(child1, child2), node.findChildren("test", EntityEvent.class)); - assertEquals(List.of(child1), node.findChildren("test", PlayerEvent.class)); - assertEquals(List.of(child3), node.findChildren("test3", EntityEvent.class)); + assertEqualsIgnoreOrder(List.of(child1, child2), node.findChildren("test", Event.class)); + assertEqualsIgnoreOrder(List.of(child1, child2), node.findChildren("test", EntityEvent.class)); + assertEqualsIgnoreOrder(List.of(child1), node.findChildren("test", PlayerEvent.class)); + assertEqualsIgnoreOrder(List.of(child3), node.findChildren("test3", EntityEvent.class)); node.removeChild(child1); - assertEquals(List.of(child2), node.findChildren("test", Event.class)); - assertEquals(List.of(child2), node.findChildren("test", EntityEvent.class)); - assertEquals(List.of(), node.findChildren("test", PlayerEvent.class)); - assertEquals(List.of(child3), node.findChildren("test3", EntityEvent.class)); + assertEqualsIgnoreOrder(List.of(child2), node.findChildren("test", Event.class)); + assertEqualsIgnoreOrder(List.of(child2), node.findChildren("test", EntityEvent.class)); + assertEqualsIgnoreOrder(List.of(), node.findChildren("test", PlayerEvent.class)); + assertEqualsIgnoreOrder(List.of(child3), node.findChildren("test3", EntityEvent.class)); } @Test @@ -72,13 +73,13 @@ public class EventNodeQueryTest { var tmp2 = EventNode.all("tmp2"); node.replaceChildren("test", tmp1); - assertEquals(List.of(child2), node.findChildren("test")); - assertEquals(List.of(tmp1), node.findChildren("tmp1")); + assertEqualsIgnoreOrder(List.of(child2), node.findChildren("test")); + assertEqualsIgnoreOrder(List.of(tmp1), node.findChildren("tmp1")); node.replaceChildren("test3", tmp2); - assertEquals(List.of(child2), node.findChildren("test")); - assertEquals(List.of(tmp1), node.findChildren("tmp1")); - assertEquals(List.of(), node.findChildren("test3")); - assertEquals(List.of(tmp2), node.findChildren("tmp2")); + assertEqualsIgnoreOrder(List.of(child2), node.findChildren("test")); + assertEqualsIgnoreOrder(List.of(tmp1), node.findChildren("tmp1")); + assertEqualsIgnoreOrder(List.of(), node.findChildren("test3")); + assertEqualsIgnoreOrder(List.of(tmp2), node.findChildren("tmp2")); } } diff --git a/src/test/java/net/minestom/server/event/EventNodeTest.java b/src/test/java/net/minestom/server/event/EventNodeTest.java index d4ce97b26..d591a20f7 100644 --- a/src/test/java/net/minestom/server/event/EventNodeTest.java +++ b/src/test/java/net/minestom/server/event/EventNodeTest.java @@ -227,6 +227,8 @@ public class EventNodeTest { public void nodeEmptyGC() { var node = EventNode.all("main"); var ref = new WeakReference<>(node); + + //noinspection UnusedAssignment node = null; waitUntilCleared(ref); } @@ -237,7 +239,39 @@ public class EventNodeTest { var ref = new WeakReference<>(node); node.addListener(EventTest.class, event -> { }); + + //noinspection UnusedAssignment node = null; waitUntilCleared(ref); } + + @Test + public void nodeChildGC() { + var node = EventNode.all("main"); + + var child = EventNode.all("child"); + var ref = new WeakReference<>(child); + child.addListener(EventTest.class, event -> { + }); + node.addChild(child); + + //noinspection UnusedAssignment + child = null; + waitUntilCleared(ref); + } + + @Test + public void nodeMapGC() { + var node = EventNode.all("main"); + + var handler = ItemStack.AIR; + var mapped = node.map(handler, EventFilter.ITEM); + var ref = new WeakReference<>(mapped); + mapped.addListener(ItemTestEvent.class, event -> { + }); + + //noinspection UnusedAssignment + mapped = null; + waitUntilCleared(ref); + } } diff --git a/src/test/java/net/minestom/server/instance/InstanceUnregisterIntegrationTest.java b/src/test/java/net/minestom/server/instance/InstanceUnregisterIntegrationTest.java index eb8f74964..63c11323b 100644 --- a/src/test/java/net/minestom/server/instance/InstanceUnregisterIntegrationTest.java +++ b/src/test/java/net/minestom/server/instance/InstanceUnregisterIntegrationTest.java @@ -1,9 +1,10 @@ package net.minestom.server.instance; +import net.minestom.server.coordinate.Pos; +import net.minestom.server.event.player.PlayerMoveEvent; +import net.minestom.server.event.player.PlayerTickEvent; import net.minestom.testing.Env; import net.minestom.testing.EnvTest; -import net.minestom.server.coordinate.Pos; -import net.minestom.server.event.player.PlayerTickEvent; import org.junit.jupiter.api.Test; import java.lang.ref.WeakReference; @@ -46,6 +47,25 @@ public class InstanceUnregisterIntegrationTest { waitUntilCleared(ref); } + @Test + public void instanceNodeGC(Env env) { + final class Game { + final Instance instance; + + Game(Env env) { + instance = env.process().instance().createInstanceContainer(); + instance.eventNode().addListener(PlayerMoveEvent.class, e -> System.out.println(instance)); + } + } + var game = new Game(env); + var ref = new WeakReference<>(game); + env.process().instance().unregisterInstance(game.instance); + + //noinspection UnusedAssignment + game = null; + waitUntilCleared(ref); + } + @Test public void chunkGC(Env env) { // Ensure that unregistering an instance does release its chunks diff --git a/testing/src/main/java/net/minestom/testing/TestUtils.java b/testing/src/main/java/net/minestom/testing/TestUtils.java index 4547a88f3..e7db258fb 100644 --- a/testing/src/main/java/net/minestom/testing/TestUtils.java +++ b/testing/src/main/java/net/minestom/testing/TestUtils.java @@ -6,18 +6,31 @@ import org.jglrxavpok.hephaistos.parser.SNBTParser; import java.io.StringReader; import java.lang.ref.WeakReference; +import java.util.Collection; +import java.util.Set; import static org.junit.jupiter.api.Assertions.*; -public class TestUtils { +public final class TestUtils { public static void waitUntilCleared(WeakReference ref) { - while (ref.get() != null) { + final int maxTries = 100; + + for (int i = 0; i < maxTries; i++) { System.gc(); + if (ref.get() == null) { + return; + } try { - Thread.sleep(50); - } catch (InterruptedException ignore) { + Thread.sleep(10); + } catch (InterruptedException e) { + throw new RuntimeException(e); } } + fail("Reference was not cleared"); + } + + public static void assertEqualsIgnoreOrder(Collection expected, Collection actual) { + assertEquals(Set.copyOf(expected), Set.copyOf(actual)); } public static void assertEqualsSNBT(String snbt, NBTCompound compound) {