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

[Kevin Rushforth]

On Sat, 24 Aug 2024 00:26:47 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>>> I'm also not sure that this is the right fix. It seems like it will fix one specific place where AWT could do an upcall on the AppKit thread after FX has detached without addressing the problem as a whole.
>>> 
>>> 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.
>>> 
>>> > Also : does AWT detach the AppKit thread from the VM on shutdown ?
>>> 
>>> I doubt it, since there isn't AFAIK an explicit toolkit shutdown in AWT like there is in FX.
>>> 
>>> > IF not why does FX have to do it ? If it does, then could FX also crash if FX is embedded in an AWT app.
>>> 
>>> FX only detaches the AppKit thread when it is the NSApplication owner, and thus attached the thread in the first place. In the case of FX embedded in an AWT app, it won't detach the thread.
>> 
>> I didn't expect it to, but I was trying to understand why an AWT exit doesn't cause FX to crash. System.exit() is the AWT equivalent and AWT *could* have a shutdown hook to do the same. It just happens not to.
>> 
>>> 
>>> 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 ? 
>> 
>>> 
>>> 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 ?
>
> We probably should replace the usage of getJNIEnv.
> 
> - JNIEnv *env = [ThreadUtilities getJNIEnv];
> + JNIEnv *env = [ThreadUtilities getJNIEnvUncached];
> 
> 
> The getJNIEnv seems unsafe if some 3p libs like FX can detach the thread.

> > 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.

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

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