On Sat, 24 Aug 2024 00:26:47 GMT, Sergey Bylokhov <s...@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