On Fri, 1 Mar 2024 18:44:17 GMT, Marius Hanl <mh...@openjdk.org> wrote:
>> This PR fixes the dialog freeze problem once and for all. >> >> This one is a bit tricky to understand, here is how it works: >> This bug happens on every platform, although the implementation of nested >> event loops differs on every platform. >> E.g. on Linux we use `gtk_main` and `gtk_main_quit`, on Windows and Mac we >> have an own implementation of a nested event loop (while loop), controlled >> by a boolean flag. >> >> Funny enough, the reason why this bug happens is always the same: Timing. >> >> 1. When we hide a dialog, `_leaveNestedEventLoop` is called. >> 2. This will call native code to get out of the nested event loop, e.g. on >> Windows we try to break out of the while loop with a boolean flag, on Linux >> we call `gtk_main_quit`. >> 3. Now, if we immediately open a new dialog, we enter a new nested event >> loop via `_enterNestedEventLoop`, as a consequence we do not break out of >> the while loop on Windows (the flag is set back again, the while loop is >> still running), and we do not return from `gtk_main` on Linux. >> 4. And this will result in the Java code never returning and calling >> `notifyLeftNestedEventLoop`, which we need to recover the UI. >> >> So it is actually not trivial to fix this problem, and we can not really do >> anything on the Java side. We may can try to wait until one more frame has >> run so that things will hopefully be right, but that sounds rather hacky. >> >> I therefore analyzed, if we even need to return from >> `_enterNestedEventLoop`. Turns out, we don't need to. >> There is a return value which we actually do not use (it does not have any >> meaning to us, other that it is used inside an assert statement). >> Without the need of a return value, we also do not need to care when >> `_enterNestedEventLoop` is returning - instead we cleanup and call >> `notifyLeftNestedEventLoop` in `_leaveNestedEventLoop`, after the native >> code was called. >> >> Lets see if this is the right approach (for all platforms). >> Testing appreciated. >> # >> - [x] Tested on Windows >> - [x] Tested on Linux >> - [x] Tested on Mac >> - [ ] Tested on iOS (although the nested event loop code is the same as for >> Mac) (I would appreciate if someone can do this as I have no access to an >> iOS device) >> - [ ] Adjust copyright >> - [ ] Write Systemtest > > Marius Hanl has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains four additional > commits since the last revision: > > - Merge branch 'master' of https://github.com/openjdk/jfx into > JDK-8285893-dialog-freezing-🥶 > - Merge remote-tracking branch 'openjfx/master' into > JDK-8285893-dialog-freezing-🥶 > - JDK-8285893: Decrement nestedEventLoopCounter in leaveNestedEventLoop > - JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen The top of the call stack when the exception is thrown: Exception in thread "JavaFX Application Thread" java.lang.IllegalStateException: Not in a nested event loop at javafx.graphics@23-internal/com.sun.glass.ui.Application.leaveNestedEventLoop(Application.java:534) at javafx.graphics@23-internal/com.sun.glass.ui.EventLoop.lambda$enter$0(EventLoop.java:122) at javafx.graphics@23-internal/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95) at javafx.graphics@23-internal/com.sun.glass.ui.mac.MacApplication._enterNestedEventLoopImpl(Native Method) at javafx.graphics@23-internal/com.sun.glass.ui.mac.MacApplication._enterNestedEventLoop(MacApplication.java:169) at javafx.graphics@23-internal/com.sun.glass.ui.Application.enterNestedEventLoop(Application.java:511) at javafx.graphics@23-internal/com.sun.glass.ui.EventLoop.enter(EventLoop.java:107) at javafx.graphics@23-internal/com.sun.javafx.tk.quantum.QuantumToolkit.enterNestedEventLoop(QuantumToolkit.java:656) at javafx.graphics@23-internal/javafx.stage.Stage.showAndWait(Stage.java:469) at ApplicationModalSampleApp2$MainFx.showModal(ApplicationModalSampleApp2.java:42) at ApplicationModalSampleApp2$MainFx.lambda$start$0(ApplicationModalSampleApp2.java:25) at javafx.base@23-internal/com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:86) at javafx.base@23-internal/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:232) at javafx.base@23-internal/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:189) at javafx.base@23-internal/com.sun.javafx.event.CompositeEventDispatcher.dispatchBubblingEvent(CompositeEventDispatcher.java:59) at javafx.base@23-internal/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58) at javafx.base@23-internal/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at javafx.base@23-internal/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56) at javafx.base@23-internal/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at javafx.base@23-internal/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56) at javafx.base@23-internal/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114) at javafx.base@23-internal/com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74) at javafx.base@23-internal/com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:49) ------------- PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1974097962
