On Thu, 22 Feb 2024 23:13:26 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 three additional > commits since the last revision: > > - 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 I see the same failure with the most recent patch as I did yesterday. I looked over the changes, and I'm not convinced that this is the right approach. One comment on your proposal: > 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. This changes one of the fundamental aspects of entering and exiting a nested event loop. This will need a lot more analysis to show why this is the right approach. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1961735156
