Just Enough Items (JEI)

Just Enough Items (JEI)

Mods
392M Downloads

[1.16.3] Huge memory leak on disconnect/reconnect

ViRb3 opened this issue ยท 4 comments

commented

Environment

Minecraft version: 1.16.3
JEI version: jei-1.16.4-7.6.0.52.jar

Reproduction

  1. Join a server
  2. Note minimum used RAM after GC in F3 menu
  3. Disconnect
  4. Repeat

Every time you join, the minimum used RAM is increased with about 400MB, depending on how many items are defined in JEI. As you can imagine, this quickly eats up all the allocated RAM and starts severely stuttering, ultimately crashing with OOM.

I took a heap dump via JProfiler and noticed this:

image

Apparently, a new instance of IngredientFilter (and RecipeManager?) is allocated on each join. I tried to trace the incoming references in hopes I will find the source, but there's just too many. If you need any other information from the heap dump, please let me know and I will do my best to provide it.

commented

It looks like this is still happening, at least for IngredientFilter:
image

This is with no mods but JEI 7.6.0.62 and Forge 35.1.10, after joining a single player world twice. The same thing happened when joining Forgecraft twice.

commented

Here is the incoming reference tree:

image

It seems to me that there are dangling InputHandler instances which in turn keep IngredientFilter instances. The InputHandler instances seem to be held by the Forge event bus system.

commented

I can confirm that this issue also happens with a completely fresh Forge instance, with just JEI installed. Each time I join a server or Single Player world, a new IngredientFilter class is spawned. I am currently sitting at 16 instances.

commented

I was correct for the most part. The root of the problem is that, among others, IngredientFilter is properly unregistered:

JustEnoughItems/src/main/java/mezz/jei/Internal.java

Lines 95 to 101 in 0c41a86

public static void setIngredientFilter(IngredientFilter ingredientFilter) {
if (Internal.ingredientFilter != null) {
EventBusHelper.unregister(Internal.ingredientFilter);
}
Internal.ingredientFilter = ingredientFilter;
EventBusHelper.register(ingredientFilter);
}

But not the listeners defined inside it:

JustEnoughItems/src/main/java/mezz/jei/ingredients/IngredientFilter.java

Lines 93 to 101 in 0c41a86

EventBusHelper.addListener(EditModeToggleEvent.class, editModeToggleEvent -> {
this.filterCached = null;
updateHidden();
});
EventBusHelper.addListener(PlayerJoinedWorldEvent.class, playerJoinedWorldEvent -> {
this.filterCached = null;
updateHidden();
});

These leftover subscriptions prevent the IngredientFilter object from being garbage collected, and results in the memory leak described above.

Now, about the fix. We could mitigate this issue by adding a shutdown hook and removing the listeners there. But this is a very error-prone task, and I can almost guarantee that more listeners will slip in the future. Therefore I propose a general solution, which will fix all of these problems and prevent them in the future. More details will follow in a PR shortly :)