Closing the window of the Clojure example causes JVM crash on MacOS

[stathissideris]

Closing the window of the Clojure example causes JVM crash on MacOS. I ran it with clj -J-XstartOnFirstThread -M -m lwjgl.main. Log attached.

hs_err_pid6917.log

[tonsky]

Thanks! I’ll look into this

[tonsky]

Sorry, I can’t repro. Are you running a modified example? How are you closing the window? By clicking on red traffic light? Which macOS version are you on?

[phronmophobic]

I had the same problem. I think the issue is this line: https://github.com/JetBrains/skija/blob/master/examples/clojure/src/lwjgl/main.clj#L71 (.free (GLFW/glfwSetErrorCallback nil))

The crash seemed to disappear after changing it to (GLFW/glfwSetErrorCallback nil). Intuitively, it seems like the free call should be invoked on the error callback created during initialization.

[tonsky]

This goes directly from LWJGL example: https://www.lwjgl.org/guide Also, this line didn’t existed on Nov 26 when this issue was created :)

[phronmophobic]

Makes sense. I no longer have that line and am still getting intermittent crashes after closing windows.

[phronmophobic]

This is the crashed thread report:

Thread 46 Crashed:: Java: Cleaner-0
0   libsystem_kernel.dylib          0x00007fff6782133a __pthread_kill + 10
1   libsystem_pthread.dylib         0x00007fff678dde60 pthread_kill + 430
2   libsystem_c.dylib               0x00007fff677a8808 abort + 120
3   libjvm.dylib                    0x000000010eff2976 os::abort(bool, void*, void const*) + 22
4   libjvm.dylib                    0x000000010f141f29 VMError::report_and_die(int, char const*, char const*, __va_list_tag*, Thread*, unsigned char*, void*, void*, char const*, int, unsigned long) + 2889
5   libjvm.dylib                    0x000000010f1413bb VMError::report_and_die(Thread*, unsigned int, unsigned char*, void*, void*, char const*, ...) + 155
6   libjvm.dylib                    0x000000010f141fff VMError::report_and_die(Thread*, unsigned int, unsigned char*, void*, void*) + 33
7   libjvm.dylib                    0x000000010eff684b JVM_handle_bsd_signal + 754
8   libjvm.dylib                    0x000000010eff43d8 signalHandler(int, __siginfo*, void*) + 45
9   libsystem_platform.dylib        0x00007fff678d25fd _sigtramp + 29
10  ???                             0x00007fe8f40aa3f0 0 + 140638503478256
11  libskija.dylib                  0x000000014173f4bb GrGLBuffer::GrGLBuffer(GrGLGpu*, unsigned long, GrGpuBufferType, GrAccessPattern, void const*) + 203
12  libskija.dylib                  0x000000014173f38f GrGLBuffer::Make(GrGLGpu*, unsigned long, GrGpuBufferType, GrAccessPattern, void const*) + 111
13  libskija.dylib                  0x000000014174e512 GrGLGpu::onCreateBuffer(unsigned long, GrGpuBufferType, GrAccessPattern, void const*) + 18
14  libskija.dylib                  0x00000001415ed20c GrGpu::createBuffer(unsigned long, GrGpuBufferType, GrAccessPattern, void const*) + 204
15  libskija.dylib                  0x0000000141610128 GrResourceProvider::createPatternedIndexBuffer(unsigned short const*, int, int, int, GrUniqueKey const*) + 88
16  libskija.dylib                  0x000000014161047e GrResourceProvider::createNonAAQuadIndexBuffer() + 46
17  libskija.dylib                  0x000000014168245b GrResourceProvider::refNonAAQuadIndexBuffer() + 43
18  libskija.dylib                  0x000000014166ee4d GrAtlasTextOp::onPrepareDraws(GrMeshDrawOp::Target*) + 573
19  libskija.dylib                  0x00000001415f4704 GrOpsTask::onPrepare(GrOpFlushState*) + 468
20  libskija.dylib                  0x0000000141607844 GrRenderTask::prepare(GrOpFlushState*) + 100
21  libskija.dylib                  0x00000001415e53ee GrDrawingManager::executeRenderTasks(int, int, GrOpFlushState*, int*) + 110
22  libskija.dylib                  0x00000001415e4e9e GrDrawingManager::flush(SkSpan<GrSurfaceProxy*>, SkSurface::BackendSurfaceAccess, GrFlushInfo const&, GrBackendSurfaceMutableState const*) + 2126
23  libskija.dylib                  0x00000001415db3d5 GrDirectContext::~GrDirectContext() + 117
24  libskija.dylib                  0x00000001415db54e GrDirectContext::~GrDirectContext() + 14
25  libskija.dylib                  0x00000001416d2b2f SkGpuDevice::~SkGpuDevice() + 95
26  libskija.dylib                  0x0000000141381038 SkCanvas::internalRestore() + 664
27  libskija.dylib                  0x0000000141380d43 SkCanvas::~SkCanvas() + 131
28  libskija.dylib                  0x00000001413810ae SkCanvas::~SkCanvas() + 14
29  libskija.dylib                  0x00000001416dfa30 SkSurface_Gpu::~SkSurface_Gpu() + 48

The crash isn't always the same thread, but it always has the same stack trace. I'm happy to paste more examples if that would be helpful.

[tonsky]

Do you have it on versions after 0.6.37 btw?

[phronmophobic]

I'm currently running version 0.6.45.

[phronmophobic]

I think I was able to narrow down the issue.

The issue happens when the context created by (DirectContext/makeGL) is garbage collected after all the glfw stuff has been cleaned up.

I created a small repro that's very similar to the example code and is available at https://github.com/phronmophobic/skija-macosx-crash-repro.

Repro steps:

skija-macosx-crash-repro$ clojure
Clojure 1.10.2
user=> (require 'com.phronemophobic.skija-macosx-crash-repro)
nil
user=> (com.phronemophobic.skija-macosx-crash-repro/run false)
nil
user=> (System/gc)
nil
user=> #
# A fatal error has been detected by the Java Runtime Environment

Ideally, the example code would work without crashing, but the crash can be prevented by manually calling .close on the context before cleaning up glfw like so, https://github.com/phronmophobic/skija-macosx-crash-repro/blob/main/src/com/phronemophobic/skija_macosx_crash_repro.clj#L75.

[phronmophobic]

It seems like the return values from BackendRenderTarget/makeGL and Surface/makeFromBackendRenderTarget also have the same problem and need to be closed manually to prevent crashes.

[tonsky]

(com.phronemophobic.skija-macosx-crash-repro/run false) is blocking for me, but closing OpenGL DirectContext before you de-initialize OpenGL itself seems reasonable?

[phronmophobic]

Thanks for looking into it.

Closing resources when they're no longer needed seems reasonable. The changes I would be interested in are updating examples and documentation that show and describe correct behavior.

Additionally, I would probably want resources that might crash the JVM when cleaned up at the wrong time to no longer be autocloseable. As a user, I'd rather have a memory leak than have a sporadic crash that's difficult to debug.

The most preferable solution would be some kind of weak reference that automatically cleans things up and doesn't crash. Potentially, it could also print an error message that says that I wasn't cleaning up after myself.

[tonsky]

I updated the Clojure example, thanks.

The most preferable solution would be some kind of weak reference that automatically cleans things up and doesn't crash.

That’s exactly how this is set up. The problem is that global GL state is managed externally to Skija resources. All Skija resources clean up cleanly, but if you closed GL context before that there’s no way of knowing or reporting that.