On Mon, 15 Jul 2024 19:36:43 GMT, Kevin Rushforth <[email protected]> wrote:
> This PR solves two related bugs that are caused by events being delivered
> while the FX runtime is in the process of shutting down. These bugs are
> related enough that I think they need to be addressed at the same time. While
> debugging and developing the test, I saw one or the other or both in various
> test runs. The system test included with this program verifies that there is
> no crash and no exception, so will catch either failure mode.
>
> This can happen when there is a full-screen window showing during shutdown on
> macOS, since full screen enter / exit uses a nested event loop that affects
> the order of operation. This could theoretically happen in other cases, thus
> the fix does not involve checking whether we are in full-screen or whether
> there is a nested event loop.
>
> The fix does the following:
>
> 1. `GlassWindow+Overrides.m`: Check that the JVM is still running before
> calling the Java notification method in `windowDidResignKey` and
> `windowShouldClose`. This matches what is already done for other similar
> methods in this class. This is the fix for JDK-8335630.
> 2. `Window.java` and `View.java`: Check whether the Application instance is
> null in the event notification callbacks, and skip handling the event if it
> is. This is the fix for JDK-8299738.
>
> Note that the fix for 2 is in platform-independent code. This should be fine,
> since we would want to skip the event handling callback on any other platform
> if it were done during shutdown, in order to avoid the ISE.
>
> I have included a system test that passes on all platforms. On macOS, the
> test will fail without the fix and pass with the fix.
>
> Details of the fix are below:
>
> `Platform::exit` is called by the application to shutdown the JavaFX toolkit.
> The implementation of `Platform::exit` calls `Toolkit::exit` on the JavaFX
> Application thread, which in turn calls glass `Application::terminate` to
> shut it down. `Application::terminate` will close all the native windows, and
> call `finishTerminating` to terminate the native event loop and detach the
> thread from the JVM. This is asynchronous, at least on macOS, and will
> schedule the termination by posting a native event to the event loop.
>
> on macOS 13, and sometimes on macOS 14, a Window or View event handler can be
> called between `Toolkit::exit`/`Application::terminate` (which sets the
> Toolkit's cached thread to null and the glass Application instance to null)
> and the JVM shutdown. This will make a JNI upcall to the appropriate Window
> or View `handleXxxxx` method, which...
The fix produces no exception and no crash on macOS 14.5 running on M1.
modules/javafx.graphics/src/main/java/com/sun/glass/ui/View.java line 533:
> 531: private boolean shouldHandleEvent() {
> 532: // Don't send any more events if the application has shutdown
> 533: if (Application.GetApplication() == null) {
Should we declare `Application.application` field as volatile?
I see interleaved multi-threaded access pattern:
thread=JavaFX-Launcher
CREATE thread=JavaFX-Launcher
thread=JavaFX-Launcher
thread=Thread-2
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX-Launcher
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX-Launcher
thread=JavaFX Application Thread
thread=PulseTimer-CVDisplayLink thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=QuantumRenderer-0
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=PulseTimer-CVDisplayLink thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX-Launcher
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX-Launcher
thread=JavaFX-Launcher
thread=JavaFX-Launcher
thread=QuantumRenderer-0
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
thread=JavaFX Application Thread
FINISH thread=JavaFX Application Thread
It might be ok though, since both creation and setting to null is done by the
FX App thread.
-------------
Marked as reviewed by angorya (Reviewer).
PR Review: https://git.openjdk.org/jfx/pull/1506#pullrequestreview-2178760909
PR Review Comment: https://git.openjdk.org/jfx/pull/1506#discussion_r1678448367
