MightyPirates / TIS-3D

TIS-100 inspired low-tech computing in Minecraft.
https://www.curseforge.com/minecraft/mc-mods/tis-3d
Other
108 stars 35 forks source link

[1.14-fabric] DisplayModule Finalization Crash #93

Closed max-kamps closed 4 years ago

max-kamps commented 5 years ago

Sometimes the following crash occurs:

FATAL ERROR in native method: Thread[Finalizer,8,system]: No context is current or a function that is not available in the current context was called. The JVM will abort execution.
    at org.lwjgl.opengl.GL11C.nglDeleteTextures(Native Method)
    at org.lwjgl.opengl.GL11C.glDeleteTextures(GL11C.java:712)
    at org.lwjgl.opengl.GL11.glDeleteTextures(GL11.java:2535)
    at com.mojang.blaze3d.platform.GlStateManager.deleteTexture(GlStateManager.java:477)
    at com.mojang.blaze3d.platform.TextureUtil.releaseTextureId(TextureUtil.java:35)
    at li.cil.tis3d.common.module.DisplayModule.deleteTexture(DisplayModule.java:259)
    at li.cil.tis3d.common.module.DisplayModule.finalize(DisplayModule.java:117)
    at java.lang.System$2.invokeFinalize(System.java:1270)
    at java.lang.ref.Finalizer.runFinalizer(Finalizer.java:102)
    at java.lang.ref.Finalizer.access$100(Finalizer.java:34)
    at java.lang.ref.Finalizer$FinalizerThread.run(Finalizer.java:217)

I can't reproduce it consistently, but seems to be caused by the fact that the DisplayModule finalizer assumes a GL context is bound, which isn't always the case. It's also possible that the finalizer thread just never has a bound GL context at all, although in that case I don't know why it only crashes sometimes and not every time.

Cheers!

max-kamps commented 5 years ago

Oh, and almost forgot about this:

-- System Details --
Details:
    Minecraft Version: 1.14
    Operating System: Linux (amd64) version 5.0.9-arch1-1-ARCH
    Java Version: 1.8.0_212, Oracle Corporation
    Java VM Version: OpenJDK 64-Bit Server VM (mixed mode), Oracle Corporation
    Memory: 502897984 bytes (479 MB) / 1751261184 bytes (1670 MB) up to 6335102976 bytes (6041 MB)
    JVM Flags: 4 total; -Xmx6G -XX:+UseConcMarkSweepGC -XX:-UseAdaptiveSizePolicy -Xmn1G
    Fabric Mods: 
        autoconfig1: 1.0.0+mc1.14
        beachslimes: 0.3.0+mc1.14-pre4
        blockus: 1.1.6
        carvemelon: 1.2.0
        chunkactivator: 0.4.0+mc1.14-pre4
        cloth-config: 0.2.1+build.14
        composableautomation: 0.2.0+mc1.14
        cotton: 0.6.1+1.14
        fabric: 0.2.7+build.127
        fabric-language-kotlin: 1.3.30+build.2
        fabricloader: 0.4.4+build.138
        harvest: 1.0.4
        leadvillagers: 0.3.0+mc1.14-pre4
        mambience: 0.4.2
        proletarian: 0.6.0+mc1.14-pre4
        tis3d: 1.5.0
        towelette: 1.5.2
    Launched Version: fabric-loader-0.4.4+build.138-1.14+build.3
    LWJGL: 3.2.1 build 12
    OpenGL: GeForce GTX 750 Ti/PCIe/SSE2 GL version 4.6.0 NVIDIA 418.56, NVIDIA Corporation
    GL Caps: Using GL 1.3 multitexturing.
Using GL 1.3 texture combiners.
Using framebuffer objects because OpenGL 3.0 is supported and separate blending is supported.
Shaders are available because OpenGL 2.1 is supported.
VBOs are available because OpenGL 1.5 is supported.

    Using VBOs: Yes
    Is Modded: Definitely; Client brand changed to 'fabric'
    Type: Client (map_client.txt)
    Resource Packs: vanilla, file/Redstone Wires
    Current Language: English (US)
    CPU: 4x Intel(R) Core(TM) i5-4460 CPU @ 3.20GHz
fnuecke commented 5 years ago

Makes sense. I believe this workaround isn't even necessary anymore. @asiekierka, do you remember if there was a reason for this besides the missing onChunkUnload (which now exists again via mixin)? Otherwise I'll just remove the finalize.

asiekierka commented 5 years ago

It's a good failsafe to have in the event that the module is not unloaded correctly, for whatever reason, to not keep GPU residue.

What I would do is call this in some kind of client-side proxy which then queues it onto MinecraftClient.getInstance()'s task queue. This way, it will always run on the correct thread.

fnuecke commented 5 years ago

Yeah, I was thinking of doing something along the lines of what the message handlers do (which is the task queue thing), but wasn't quite convinced this wouldn't cause its own set of problems :x It being a failsafe is a good point tho, will give it a shot and see how it goes.

asiekierka commented 5 years ago

@max-kamps Could you try using the latest build?

fnuecke commented 4 years ago

Presumably the fix worked.