Fix event node child/map leak

Signed-off-by: TheMode <themode@outlook.fr>
This commit is contained in:
TheMode 2022-10-27 22:32:22 +02:00
parent b4af87b8a4
commit e11d15af0d
5 changed files with 102 additions and 27 deletions

View File

@ -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<T extends Event> implements EventNode<T> {
private final Map<Class, Handle<T>> handleMap = new ConcurrentHashMap<>();
final Map<Class<? extends T>, ListenerEntry<T>> listenerMap = new ConcurrentHashMap<>();
final Set<EventNodeImpl<T>> children = new CopyOnWriteArraySet<>();
final Map<Object, EventNodeImpl<T>> mappedNodeCache = new WeakHashMap<>();
final Map<Object, EventNodeImpl<T>> registeredMappedNode = new WeakHashMap<>();
final Set<EventNodeImpl<T>> children = Collections.newSetFromMap(Caffeine.newBuilder()
.weakKeys().<EventNodeImpl<T>, Boolean>build().asMap());
final Map<Object, EventNodeImpl<T>> mappedNodeCache = Caffeine.newBuilder()
.weakKeys().weakValues().<Object, EventNodeImpl<T>>build().asMap();
final Map<Object, EventNodeImpl<T>> registeredMappedNode = Caffeine.newBuilder()
.weakKeys().weakValues().<Object, EventNodeImpl<T>>build().asMap();
final String name;
final EventFilter<T, ?> filter;
@ -50,6 +54,7 @@ non-sealed class EventNodeImpl<T extends Event> implements EventNode<T> {
@Override
public <E extends T> @NotNull List<EventNode<E>> findChildren(@NotNull String name, Class<E> eventType) {
synchronized (GLOBAL_CHILD_LOCK) {
final Set<EventNode<T>> children = getChildren();
if (children.isEmpty()) return List.of();
List<EventNode<E>> result = new ArrayList<>();
for (EventNode<T> child : children) {
@ -70,6 +75,7 @@ non-sealed class EventNodeImpl<T extends Event> implements EventNode<T> {
@Override
public <E extends T> void replaceChildren(@NotNull String name, @NotNull Class<E> eventType, @NotNull EventNode<E> eventNode) {
synchronized (GLOBAL_CHILD_LOCK) {
final Set<EventNode<T>> children = getChildren();
if (children.isEmpty()) return;
for (EventNode<T> child : children) {
if (equals(child, name, eventType)) {
@ -85,6 +91,7 @@ non-sealed class EventNodeImpl<T extends Event> implements EventNode<T> {
@Override
public void removeChildren(@NotNull String name, @NotNull Class<? extends T> eventType) {
synchronized (GLOBAL_CHILD_LOCK) {
final Set<EventNode<T>> children = getChildren();
if (children.isEmpty()) return;
for (EventNode<T> child : children) {
if (equals(child, name, eventType)) {

View File

@ -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"));
}
}

View File

@ -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);
}
}

View File

@ -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

View File

@ -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 <T> void assertEqualsIgnoreOrder(Collection<T> expected, Collection<? extends T> actual) {
assertEquals(Set.copyOf(expected), Set.copyOf(actual));
}
public static void assertEqualsSNBT(String snbt, NBTCompound compound) {