On Tue, 9 Jan 2024 21:45:33 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

As I was afraid of, this change causes many test failures (see below for just 
one example). I think a different approach is needed, one that preserves the 
synchronous nature of these methods.


NestedEventLoopTest > testCanEnterAndExitNestedEventLoop FAILED
    java.lang.AssertionError
        at org.junit.Assert.fail(Assert.java:87)
        at org.junit.Assert.assertTrue(Assert.java:42)
        at org.junit.Assert.assertFalse(Assert.java:65)
        at org.junit.Assert.assertFalse(Assert.java:75)
        at 
test.javafx.stage.NestedEventLoopTest.lambda$testCanEnterAndExitNestedEventLoop$2(NestedEventLoopTest.java:120)
        at test.util.Util.lambda$submit$0(Util.java:108)
        at 
javafx.graphics@23-ea/com.sun.javafx.application.PlatformImpl.lambda$runLater$10(PlatformImpl.java:456)
        at 
java.base/java.security.AccessController.doPrivileged(AccessController.java:400)
        at 
javafx.graphics@23-ea/com.sun.javafx.application.PlatformImpl.lambda$runLater$11(PlatformImpl.java:455)
        at 
javafx.graphics@23-ea/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95)
        at 
javafx.graphics@23-ea/com.sun.glass.ui.mac.MacApplication._enterNestedEventLoopImpl(Native
 Method)
        at 
javafx.graphics@23-ea/com.sun.glass.ui.mac.MacApplication._enterNestedEventLoop(MacApplication.java:169)
        at 
javafx.graphics@23-ea/com.sun.glass.ui.Application.enterNestedEventLoop(Application.java:512)
        at 
javafx.graphics@23-ea/com.sun.glass.ui.EventLoop.enter(EventLoop.java:107)
        at 
javafx.graphics@23-ea/com.sun.javafx.tk.quantum.QuantumToolkit.enterNestedEventLoop(QuantumToolkit.java:656)
        at 
javafx.graphics@23-ea/javafx.application.Platform.enterNestedEventLoop(Platform.java:310)
        at 
test.javafx.stage.NestedEventLoopTest.lambda$testCanEnterAndExitNestedEventLoop$0(NestedEventLoopTest.java:112)

@Maran23 I see from the description that you tested this on mac. How did you 
test it? Did you run with `-PFULL_TEST=true`?

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

Changes requested by kcr (Lead).

PR Review: https://git.openjdk.org/jfx/pull/1324#pullrequestreview-1896186878
PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1959717323

Reply via email to