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 first `runLater` block is being invoked by the first nested event loop; 
that loop is on the call stack. In that block you call `exitNestedEventLoop` 
but don't allow the stack to unwind before calling `enterNestedEventLoop`. So 
on the stack the new event loop is indeed nested inside the previous one (the 
previous one is marked as LEAVING but it's still doing something). Since this 
is a stack there's only one way to unwind it which is in reverse order. The 
output I would expect would be:


enter1
exit1
enter2
exit2
456
123


There is that bug in the invokeLaterDispatcher which will prevent the second 
`runLater` block from executing. And the documentation doesn't really address 
this case but it should.

By the way, when I grab the code in the original bug report and run it agains 
this PR the innermost modal is not blocked and everything seems fine. But if I 
press the Close button exceptions start getting thrown. This is happening on 
both Mac and Windows.

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

PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1973953042

Reply via email to