RFR: 8339178: [macos] Swing InterOp Platform.exit() crash

[Kevin Rushforth]

On Mon, 26 Aug 2024 21:57:51 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> A Swing / FX interop app will crash if an application creates a new AWT / Swing window after calling Platform.exit. The root cause of the crash is that AWT caches the JNI env pointer for the AppKit thread, and assumes that it is valid for the life of the application. In the case where JavaFX is the owner of the NSApplication, we detach the AppKit thread from the JVM, after which AWT's env pointer is no longer valid. AWT will therefore crash the next time it does a JNI upcall.
>> 
>> This PR fixes the crash by leaving the macOS AppKit thread attached to the JVM after the JavaFX main event loop terminates. This requires attaching the AppKit thread to the JVM as a daemon thread when JavaFX is the NSApplication owner, matching what AWT does when it is the owner. In order to prevent a JavaFX application from exiting prematurely, create a non-daemon "KeepAlive" thread that can be terminated when the FX toolkit exits. This also solves a somewhat-related problem where the JavaFX toolkit will exit prematurely if the AWT toolkit is started first, and all AWT windows are disposed.
>> 
>> This fix is in addition to the AWT fix: [JDK-8190329](https://bugs.openjdk.org/browse/JDK-8190329) / openjdk/jdk#20688. Either the AWT fix or the JavaFX fix is sufficient to avoid this specific problem, but there is value in fixing it in both places, so I cloned the AWT bug to create a JavaFX bug that we can use for this PR.
>> 
>> Summary of the changes:
>> 
>> * Attach the AppKit thread to the JVM as a daemon
>> * Do not detach the thread when the FX main event loop terminates
>> * Create and start a KeepAlive thread in MacApplication, when the FX toolkit starts
>> * Terminate the KeepAlive thread when the FX toolkit finishes
>> 
>> Testing:
>> 
>> I created automated systems tests from the manual test programs in the bugs as well as a test to ensure that we don't regress and exit prematurely in the pure JavaFX case (which would happen without the KeepAlive thread). Two of the new tests fail on macOS without the fix and pass with the fix. All three pass on all platforms with the fix.
>
> modules/javafx.graphics/src/main/java/com/sun/glass/ui/mac/MacApplication.java line 81:
> 
>> 79:         thr.setName("JavaFX-KeepAlive");
>> 80:         thr.setDaemon(false);
>> 81:         thr.start();
> 
> is it possible to reuse some other thread for this purpose?
> this may not be an issue when virtual threads take hold, but right now threads are precious native resource.

No. The whole reason we are doing this is that there are no other non-daemon threads. I updated the docs to make this more clear. Anyway, threads aren't _that_ scarce as long as you don't create hundreds of them.

Loosely related: This reminds me that `javafx.application.Application.launch` does create an unnecessary thread (on all platforms). There is a long-standing issue to get rid of it:

[JDK-8090323](https://bugs.openjdk.org/browse/JDK-8090323): Consider eliminating the JavaFX-Launcher Thread

It isn't high priority, but would be good cleanup.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1545#discussion_r1732997480