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

[Kevin Rushforth]

On Sat, 24 Aug 2024 21:16:00 GMT, Phil Race <prr at openjdk.org> wrote:

>>> > Another possible fix would be to have AWT not use the cached JNI Env when it is not the NSApplication owner. I don't know how viable this fix would be, or whether it is the right fix, but it would prevent the crash in all possible places.
>>> 
>>> That sounds like lots of places would need to be updated to do this.
>> 
>> No, only in one place. The idea I had -- and did a quick prototype of -- was to change the implementation of `[ThreadUtilities getJNIEnv]` to check a flag to see whether or not AWT was the owner of the NSApplication and, if not, effectively do the same thing as `getJNIEnvUncached`. What that would then do would be to re-attach the AppKit thread to the JVM if FX had detached it. This will avoid the crash in all places without having to change the callers of `getJNIEnv`
>> 
>>  > We could explore what it would take for FX to not detach the AppKit thread from Java. At a minimum, it would require attaching the AppKit thread as a daemon (which AWT already does), and then creating a non-daemon Java thread that we would then terminate at FX toolkit shutdown. This would need to be tested carefully, but might be the cleanest solution.
>>> 
>>> I would like to see that explored. What does SWT do ?
>> 
>> I'll explore this for FX. I think it will be pretty easy. No idea what SWT does.
>> 
>>> > One thing to note is that anything we do here will be about avoiding a crash in AWT, not about having the UI remain functional. In this case FX is the "owner" of the native Window system event loop (and NSApplication on macOS). Applications should not shutdown the FX toolkit (meaning, not call `Platform.exit`) and expect AWT to continue to work.
>>> 
>>> I guess Platform.exit for FX embedded inside AWT is a no-op ?
>> 
>> Not a no-op, since it still shuts down the FX toolkit and renderer, but it doesn't detach the AppkitThread from the JVM.
>
>> > > Another possible fix would be to have AWT not use the cached JNI Env when it is not the NSApplication owner. I don't know how viable this fix would be, or whether it is the right fix, but it would prevent the crash in all possible places.
>> > 
>> > 
>> > That sounds like lots of places would need to be updated to do this.
>> 
>> No, only in one place. The idea I had -- and did a quick prototype of -- was to change the implementation of `[ThreadUtilities getJNIEnv]` to check a flag to see whether or not AWT was the owner of the NSApplication and, if not, effectively do the same thing as `getJNIEnvUncached`. What that would then do would be to re-attach the AppKit thread to the JVM if FX had detached it. This will avoid the crash in all places without having to change the callers of `getJNIEnv`
>  
> I've looked and I think that would work. Minimal overhead in the 99.99% "non-embedded" case.

Here is a Draft PR that fixes this in FX, if we decide to go that route: openjdk/jfx#1545

I added three new regression tests. Two of them (including one that reproduces this crash) fail without the fix. All three pass with the fix.

If you prefer, we can continue to explore whether or not to fix this in AWT in addition to or instead of the AWT fix.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20688#discussion_r1731835683