RFR: 8285893: Hiding dialog and showing new one causes dialog to be frozen [v8]
Marius Hanl
mhanl at openjdk.org
Thu May 23 12:48:22 UTC 2024
> 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.~
> See below for more information. To recover the UI (and in general nested nested event loops ;) we need to set a flag for the `InvokeLaterDispatcher`.
>
> 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)
> - [x] Adjust copyright
> - [x] Write Systemtest
> - [x] Document the new behaviour - in general, there should be more information what to expect
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 11 additional commits since the last revision:
- Merge branch 'master' of https://github.com/openjdk/jfx into JDK-8285893-dialog-freezing-🥶
- Integrate changes as outline by beldenfox
- test for multiple nested event loops
- try leave outer loop after finishing inner loop
- update copyright
- trigger an outer nested event loop leave attempt
- do not attempt to leave eventloop after leaving another eventloop
- 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
- ... and 1 more: https://git.openjdk.org/jfx/compare/ea0d1a15...ec43f2e3
-------------
Changes:
- all: https://git.openjdk.org/jfx/pull/1324/files
- new: https://git.openjdk.org/jfx/pull/1324/files/bba7fc23..ec43f2e3
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jfx&pr=1324&range=07
- incr: https://webrevs.openjdk.org/?repo=jfx&pr=1324&range=06-07
Stats: 86781 lines in 1031 files changed: 46446 ins; 20952 del; 19383 mod
Patch: https://git.openjdk.org/jfx/pull/1324.diff
Fetch: git fetch https://git.openjdk.org/jfx.git pull/1324/head:pull/1324
PR: https://git.openjdk.org/jfx/pull/1324
More information about the openjfx-dev
mailing list