sp614x / optifine

1.79k stars 418 forks source link

Leaking resource on startup #4678

Closed Chocohead closed 4 years ago

Chocohead commented 4 years ago

If you run the game with the debug logger turned on leaked resources that are loaded through FallbackResourcemanager (MCP)/NamespaceResourceManager (Yarn) will be printed to the log. Doing so with Optifine reveals a couple of places where InputStreams are missed from being closed.

OptiFine Version

1.16.2-OptFine_HD_U_G3_pre2

Sample Logs

All using Intermediary names but it's clear enough what's happening without recognising the class IDs

Translation loading ``` [Finalizer/WARN]: Leaked resource: 'minecraft:optifine/lang/en_us.lang' loaded from pack: 'Default' java.lang.Exception at net.minecraft.class_3294$class_3295.(class_3294.java:110) at net.minecraft.class_3294.method_14476(class_3294.java:89) at net.minecraft.class_3294.method_14486(class_3294.java:63) at net.minecraft.class_3304.method_14486(class_3304.java:64) at net.optifine.Lang.loadResources(Lang.java:128) at net.minecraft.class_1078.method_4675(class_1078.java:50) at net.minecraft.class_1076.method_14491(class_1076.java:65) at net.minecraft.class_4013.method_29490(class_4013.java:15) at java.util.concurrent.CompletableFuture.uniRun(Unknown Source) at java.util.concurrent.CompletableFuture$UniRun.tryFire(Unknown Source) at java.util.concurrent.CompletableFuture$Completion.run(Unknown Source) at net.minecraft.class_4010.method_18353(class_4010.java:40) at net.minecraft.class_4014.method_18365(class_4014.java:71) at net.minecraft.class_1255.method_18859(class_1255.java:172) at net.minecraft.class_4093.method_18859(class_4093.java:23) at net.minecraft.class_1255.method_16075(class_1255.java:134) at net.minecraft.class_1255.method_5383(class_1255.java:115) at net.minecraft.class_310.method_1523(class_310.java:990) at net.minecraft.class_310.method_1514(class_310.java:656) at net.minecraft.client.main.Main.main(Main.java:215) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at java.lang.reflect.Method.invoke(Unknown Source) at net.fabricmc.loader.game.MinecraftGameProvider.launch(MinecraftGameProvider.java:192) at net.fabricmc.loader.launch.knot.Knot.init(Knot.java:140) at net.fabricmc.loader.launch.knot.KnotClient.main(KnotClient.java:26) ```
Texture Atlas Loading ``` [Finalizer/WARN]: Leaked resource: 'minecraft:textures/item/brick.png' loaded from pack: 'Default' java.lang.Exception at net.minecraft.class_3294$class_3295.(class_3294.java:110) at net.minecraft.class_3294.method_14476(class_3294.java:89) at net.minecraft.class_3294.method_14486(class_3294.java:63) at net.minecraft.class_3304.method_14486(class_3304.java:64) at net.minecraft.class_1059.detectMinimumSpriteSize(class_1059.java:809) at net.minecraft.class_1059.detectMaxMipmapLevel(class_1059.java:780) at net.minecraft.class_1059.method_18163(class_1059.java:276) at net.minecraft.class_1088.processLoading(class_1088.java:256) at net.minecraft.class_1088.(class_1088.java:170) at net.minecraft.class_1092.method_18178(class_1092.java:52) at net.minecraft.class_1092.method_18789(class_1092.java:19) at net.minecraft.class_4080.method_18791(class_4080.java:11) at java.util.concurrent.CompletableFuture$AsyncSupply.run(Unknown Source) at net.minecraft.class_4010.method_18357(class_4010.java:35) at net.minecraft.class_4014.method_18371(class_4014.java:64) at java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(Unknown Source) at java.util.concurrent.ForkJoinTask.doExec(Unknown Source) at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(Unknown Source) at java.util.concurrent.ForkJoinPool.runWorker(Unknown Source) at java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source) ```
Better Grass Config ``` [Finalizer/WARN]: Leaked resource: 'minecraft:optifine/bettergrass.properties' loaded from pack: 'Default' java.lang.Exception at net.minecraft.class_3294$class_3295.(class_3294.java:110) at net.minecraft.class_3294.method_14476(class_3294.java:89) at net.minecraft.class_3294.method_14486(class_3294.java:63) at net.minecraft.class_3304.method_14486(class_3304.java:64) at net.optifine.Config.getResourceStream(Config.java:1287) at net.optifine.Config.getResourceStream(Config.java:1278) at net.optifine.BetterGrass.loadProperties(BetterGrass.java:140) at net.optifine.BetterGrass.updateIcons(BetterGrass.java:81) at net.optifine.util.TextureUtils.registerCustomSprites(TextureUtils.java:455) at net.minecraft.class_1059.method_18163(class_1059.java:265) at net.minecraft.class_1088.processLoading(class_1088.java:256) at net.minecraft.class_1088.(class_1088.java:170) at net.minecraft.class_1092.method_18178(class_1092.java:52) at net.minecraft.class_1092.method_18789(class_1092.java:19) at net.minecraft.class_4080.method_18791(class_4080.java:11) at java.util.concurrent.CompletableFuture$AsyncSupply.run(Unknown Source) at net.minecraft.class_4010.method_18357(class_4010.java:35) at net.minecraft.class_4014.method_18371(class_4014.java:64) at java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(Unknown Source) at java.util.concurrent.ForkJoinTask.doExec(Unknown Source) at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(Unknown Source) at java.util.concurrent.ForkJoinPool.runWorker(Unknown Source) at java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source) ```
sp614x commented 4 years ago

Fixed in pre3.

Chocohead commented 4 years ago

The fix seems to have worked for all the normal textures (using 1.16.2-OptFine_HD_U_G3), but ones with meta files are still leaving warnings in the logs:

Texture Atlas Loading ``` [Finalizer/WARN]: Leaked resource: 'minecraft:textures/block/soul_fire_0.png.mcmeta' loaded from pack: 'Default' java.lang.Exception at net.minecraft.class_3294$class_3295.(class_3294.java:110) at net.minecraft.class_3294.method_14476(class_3294.java:89) at net.minecraft.class_3294.method_14486(class_3294.java:61) at net.minecraft.class_3304.method_14486(class_3304.java:64) at net.minecraft.class_1059.detectMinimumSpriteSize(class_1059.java:809) at net.minecraft.class_1059.detectMaxMipmapLevel(class_1059.java:780) at net.minecraft.class_1059.method_18163(class_1059.java:276) at net.minecraft.class_1088.processLoading(class_1088.java:257) at net.minecraft.class_1088.(class_1088.java:170) at net.minecraft.class_1092.method_18178(class_1092.java:52) at net.minecraft.class_1092.method_18789(class_1092.java:19) at net.minecraft.class_4080.method_18791(class_4080.java:11) at java.util.concurrent.CompletableFuture$AsyncSupply.run(Unknown Source) at net.minecraft.class_4010.method_18357(class_4010.java:35) at net.minecraft.class_4014.method_18371(class_4014.java:64) at java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(Unknown Source) at java.util.concurrent.ForkJoinTask.doExec(Unknown Source) at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(Unknown Source) at java.util.concurrent.ForkJoinPool.runWorker(Unknown Source) at java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source) ```

Little bit of an oversight on my part from what leaks, it's the IResource (MCP)/Resource (Yarn) itself which should be closed to ensure both the actual resource InputStream and meta file InputStream (if it exists) are closed. I would think this only affects SpriteAtlasTexture#detectMinimumSpriteSize as neither lang files nor the better grass config would need meta files. It would thus probably make most sense to just put res in the try which would make the previous fix closing in entirely optional:

try (Resource res = rm.getResource(locComplete)) {
...
} catch (Exception e) {
}

Also has the advantage of properly closing if an exception is throw, which the previous solution did not.