electronstudio / jaylib-ffm

Other
14 stars 2 forks source link

Shared library can't be deleted on shutdown in Windows #10

Closed bploeckelman closed 1 month ago

bploeckelman commented 2 months ago

When the shutdown hook from Util:51 runs on my Windows 11 system, it results in an AccessDeniedException as shown below, and the raylib dll that was extracted to the user's temp directory isn't deleted. When iterating on project code this results in a lot of duplicate dlls accumulating in the temp directory.

Example stacktrace:

Exception in thread "Thread-0" java.lang.RuntimeException: java.nio.file.AccessDeniedException: C:\Users\USERNAME\AppData\Local\Temp\6737253390355114103.dll
        at uk.co.electronstudio.Util$1.run(Util.java:53)
Caused by: java.nio.file.AccessDeniedException: C:\Users\USERNAME\AppData\Local\Temp\6737253390355114103.dll
        at java.base/sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:89)
        at java.base/sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:103)
        at java.base/sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:108)
        at java.base/sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:273)
        at java.base/sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:104)
        at java.base/java.nio.file.Files.delete(Files.java:1152)
        at uk.co.electronstudio.Util$1.run(Util.java:51)

I spent a bit of time digging into this to see if I could sort out the cause since it seems odd that Files.createTempFile(...) would allow a process to write a file to the temp directory but not allow the same process to delete it. While I didn't come up with a definite explanation, my guess is that the shared library isn't unloaded at the time the shutdown hook runs.

The FFM API is new for me, but reviewing the library code I wonder if a solution might involve changing raylib_h_1.LIBRARY_ARENA from Arena.ofAuto() to a confined arena that can be managed directly rather than automatically, such that it could be manually closed either as a part of the shutdown hook before trying to delete the shared lib file, or via an explicit Raylib.shutdown() call made by application code.

electronstudio commented 2 months ago

Yes could try that.

Seems this isn’t a new problem https://coderanch.com/t/324444/java/JNI-Load-Unload-library

However a further problem is that no matter what we do we won’t be able to delete the file if the JVM crashes in native code (which is not uncommon with Raylib). So perhaps we should use a fixed name for the file and overwrite the same file every run.

bploeckelman commented 2 months ago

I tested manually closing LIBRARY_ARENA in the shutdown hook: bploeckelman/jaylib-ffm a3add4e, and while it works I agree that it's not a great approach since it doesn't handle crashes.

Besides that, it's clunky because it requires another modification of raylib_h_1.java to rewrite how jextract generates the static final Arena LIBRARY_ARENA = Arena.ofAuto(); line, not just to change which factory method to call, but also to change its access to public so that it can be close()'d in Util.java. There may be a way to configure jextract to do something like this but I didn't take the time to dig into that.

Also, since raylib_h_1.LIBRARY_ARENA.close() would be called from a shutdown hook thread, using Arena.ofConfined() doesn't work since a confined arena can't be modified from a different thread, so I used Arena.ofShared() instead which is less restrictive about access from non-owning threads.

So taking all that into account I agree that the better approach is just to use a fixed filename and overwrite on extract. I might be able to get a pull request together for that, but I'm out of time today and not sure when I'll have a chance to get back to it. I'll update this issue if I manage to get that together.