Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v6]
On Wed, 6 Mar 2024 17:24:10 GMT, Martin Fox wrote: >> Marius Hanl has updated the pull request incrementally with four additional >> commits since the last revision: >> >> - test for multiple nested event loops >> - try leave outer loop after finishing inner loop >> - update copyright >> - trigger an outer nested event loop leave attempt > > The Mac is still failing the NestedEventLoop test. > > At the top of InvokeLaterDispatcher.java there's a long explanation on how > it's trying to coordinate with the event loop. To do this it needs to have an > accurate picture of what's happening at the platform level. In this PR you're > telling the dispatcher that an event loop has exited before the platform has > actually exited the loop (before enterNestedEventLoop has returned). This is > causing the dispatcher to send runLater runnables to the platform earlier > than expected. On Windows the runnables will get deferred to the next event > loop cycle but on Mac the runnables might get executed in the current event > loop cycle. I think this is one of the "native system limitations" mentioned > in the comment in InvokeLaterDispatcher.java. > > I've created a branch with my proposed fix for this problem > (https://github.com/beldenfox/jfx/tree/eventloopjam). The fix prevents the > dispatcher from jamming when the event loop it thought was leaving enters a > new loop instead. Over in the Mac Glass code I also added a comment with a > few more details on what's going on. Thanks! I feel like we are coming closer to the fix of this problem. I hopefully have time to test this much more tomorrow - I have at least "booked" myself a time slot for JavaFX. - PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1984685350
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v7]
> 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) > - [x] Adjust copyright > - [x] Write Systemtest > - [ ] Document the new behaviour - in general, there shoul be more > information what to expect Marius Hanl has updated the pull request incrementally with one additional commit since the last revision: Integrate changes as outline by beldenfox - Changes: - all: https://git.openjdk.org/jfx/pull/1324/files - new: https://git.openjdk.org/jfx/pull/1324/files/a63b5254..bba7fc23 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1324=06 - incr: https://webrevs.openjdk.org/?repo=jfx=1324=05-06 Stats: 58 lines in 8 files changed: 39 ins; 13 del; 6 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
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v6]
On Wed, 6 Mar 2024 17:24:10 GMT, Martin Fox wrote: > The Mac is still failing the NestedEventLoop test. Thanks for testing this on Mac. I always need to ask someone in order to test the changes, so really appreciating that. > I've created a branch with my proposed fix for this problem > (https://github.com/beldenfox/jfx/tree/eventloopjam). The fix prevents the > dispatcher from jamming when the event loop it thought was leaving enters a > new loop instead. Over in the Mac Glass code I also added a comment with a > few more details on what's going on. I tested your code and it looks good! So this seems to be the better approach here. While mine works for Windows and maybe Linux, it does not for Mac as you also specified. The return value of the native nested event loops are still not needed, so we can still think about keeping this change, but we should probably revert the EventLoop changes and take your fix instead. - PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1984564786
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v6]
On Tue, 5 Mar 2024 23:37:18 GMT, Marius Hanl 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) >> - [x] Adjust copyright >> - [x] Write Systemtest >> - [ ] Document the new behaviour - in general, there shoul be more >> information what to expect > > Marius Hanl has updated the pull request incrementally with four additional > commits since the last revision: > > - test for multiple nested event loops > - try leave outer loop after finishing inner loop > - update copyright > - trigger an outer nested event loop leave attempt The Mac is still failing the NestedEventLoop test. At the top of InvokeLaterDispatcher.java there's a long explanation on how it's trying to coordinate with the event loop. To do this it needs to have an accurate picture of what's happening at the platform level. In this PR you're telling the dispatcher that an event loop has exited before the platform has actually exited the loop (before enterNestedEventLoop has returned). This is causing the dispatcher to send runLater runnables to the platform earlier than expected. On Windows the runnables will get deferred to the next event loop cycle but on Mac the runnables might get executed in the current event loop cycle. I think this is one of the "native system limitations" mentioned in the comment in InvokeLaterDispatcher.java. I've created a branch with my proposed fix for this problem (https://github.com/beldenfox/jfx/tree/eventloopjam). The fix prevents the dispatcher from jamming when the event loop it thought was leaving enters a new loop instead. Over in the Mac Glass code I also added a comment with a few more details on what's going on. - PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1981400654
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v5]
On Mon, 4 Mar 2024 19:16:06 GMT, Marius Hanl 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 refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains one new > commit since the last revision: > > do not attempt to leave eventloop after leaving another eventloop I may found a solution which fixes the dialog bug and passes my test above. It behaves exactly like @beldenfox said. enter1 exit1 enter2 exit2 456 123 - PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1979817144
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v6]
> 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 incrementally with four additional commits since the last revision: - test for multiple nested event loops - try leave outer loop after finishing inner loop - update copyright - trigger an outer nested event loop leave attempt - Changes: - all: https://git.openjdk.org/jfx/pull/1324/files - new: https://git.openjdk.org/jfx/pull/1324/files/0bcda3a2..a63b5254 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1324=05 - incr: https://webrevs.openjdk.org/?repo=jfx=1324=04-05 Stats: 86 lines in 12 files changed: 71 ins; 1 del; 14 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
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v5]
> 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 refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: do not attempt to leave eventloop after leaving another eventloop - Changes: - all: https://git.openjdk.org/jfx/pull/1324/files - new: https://git.openjdk.org/jfx/pull/1324/files/508e2569..0bcda3a2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1324=04 - incr: https://webrevs.openjdk.org/?repo=jfx=1324=03-04 Stats: 10 lines in 1 file changed: 0 ins; 10 del; 0 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
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v3]
On Fri, 1 Mar 2024 23:56:19 GMT, Martin Fox wrote: > The top of the call stack when the exception is thrown: Yeah, had the same issue and fixed it locally. Now pushed. Problem as stated above remains though - PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1977265143
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v4]
> 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 incrementally with one additional commit since the last revision: do not attempt to leave eventloop after it is done - Changes: - all: https://git.openjdk.org/jfx/pull/1324/files - new: https://git.openjdk.org/jfx/pull/1324/files/a4cce087..508e2569 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1324=03 - incr: https://webrevs.openjdk.org/?repo=jfx=1324=02-03 Stats: 20 lines in 1 file changed: 10 ins; 10 del; 0 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
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v3]
On Fri, 1 Mar 2024 18:44:17 GMT, Marius Hanl 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
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v3]
On Fri, 1 Mar 2024 21:33:16 GMT, Martin Fox wrote: > The output I would expect would be: > > ``` > enter1 > exit1 > enter2 > exit2 > 456 > 123 > ``` You are right, this sounds correct. > 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. Can you post the exception? I may know which one and I already fixed one locally, testing it currently. - PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1974005439
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v3]
On Fri, 1 Mar 2024 18:44:17 GMT, Marius Hanl 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
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v3]
On Fri, 1 Mar 2024 18:44:17 GMT, Marius Hanl 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
So I could finally look into the issue. Unfortunately, the problem is much
deeper than I thought.
Consider the following testcase:
Platform.runLater(() -> {
System.out.println("exit1");
Platform.exitNestedEventLoop(this, "123");
Platform.runLater(() -> {
System.out.println("exit2");
Platform.exitNestedEventLoop("this", "456");
});
System.out.println("enter2");
Object o = Platform.enterNestedEventLoop("this");
System.out.println(o);
});
System.out.println("enter1");
Object o = Platform.enterNestedEventLoop(this);
System.out.println(o);
What I would expect:
enter1
exit1
123
enter2
exit2
456
New behaviour:
enter1
exit1
enter2
exit2
456
Old behaviour:
enter1
exit1
enter2
Application hangs (Makes sense, as it is the same logic as if we hide a dialog
and show a new one)
So it is better now, since we do not stall the application anymore, but we
never return back from the first event loop, since the native code did not
return. And I would like to fix this completely.
Maybe, the approach needs to be a little bit different.
Something like: If we enter a nested event loop WHILE we are exiting one, we
should somewhat wait for it to finish.
That is a pretty though one.
-
PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1973787728
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v3]
> 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 - Changes: - all: https://git.openjdk.org/jfx/pull/1324/files - new: https://git.openjdk.org/jfx/pull/1324/files/cb8865e8..a4cce087 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1324=02 - incr: https://webrevs.openjdk.org/?repo=jfx=1324=01-02 Stats: 1338 lines in 147 files changed: 810 ins; 156 del; 372 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
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v2]
On Wed, 28 Feb 2024 20:20:35 GMT, Martin Fox wrote: > I don't think this is a problem with the nested event loop bookkeeping. It > looks like a much simpler bug in the invokeLaterDispatcher. Well, yes and no. My testing was showing that `notifyLeftNestedEventLoop` was not even called in the finally block, so there is that. We may still have a problem at the `InvokeLaterDispatcher`, although not 100% sure since it worked good for me on Windows. But I will test it more as soon as I have more time, since I want to really check everything, so I do not miss anything here. - PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1969990780
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v2]
On Thu, 22 Feb 2024 23:13:26 GMT, Marius Hanl 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 don't think this is a problem with the nested event loop bookkeeping. It looks like a much simpler bug in the invokeLaterDispatcher. When exitNestedEventLoop is called on the innermost loop the invokeLaterDispatcher suspends operation until the loop finishes. But if you immediately start a new event loop the previous one won't finish and the dispatcher will be wedged. If you're already in a nested loop you can get into the wedged state with just two lines of code: `exitNestedEventLoop(innermost, retVal); enterNestedEventLoop(newLoop);` When the invokeLaterDispatcher is told that the innermost loop is exiting it currently sets `leavingNestedEventLoop` to true. When the dispatcher is told that a new event loop has started it is *not* clearing `leavingNestedEventLoop` but it should. Basically it should follow the same logic used in glass; leaving the innermost loop updates a boolean indicating that the loop should exit but if a new loop is started the boolean is set back to a running state since it now applies to the new loop, not the previous one. - PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1969844034
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v2]
On Thu, 22 Feb 2024 23:13:26 GMT, Marius Hanl 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 The Mac is failing on the simplest NestedEventLoop test, the one that verifies that we can enter and exit a nested loop and correctly retrieve the return value. I started looking into this but find what's happening in Glass a bit confusing. I'll take a closer look in the next day or two. I don't understand the original failure case. Events aren't blocked and you can type into the modal's text field, it just doesn't repaint. If you resize the window you will see your edits. - PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1967605288
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v2]
On Fri, 23 Feb 2024 17:39:44 GMT, Kevin Rushforth wrote: > 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. Yes, this is right and I also thought about documenting that as soon as we think that this is the right approach. I completely agree that we need to analyse this. This is not a simple change but will affect Dialogs and every custom code that uses nested event loops (e.g. I used it for blocking the UI without freezing it for data loading purposes, where we want to wait until the code returns in a non UI blicking way). Maybe there is a better way as well, I just don't figured out one. As described, right now we rely that `enterNestedEventLoop` returns always, which is not happening immediately for ALL platforms, even though Platform has another implementation. Which makes me think the current approach has a flaw, resulting in this behaviour. > 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. Thanks for testing. As soon as my colleague is available, I will debug this issue. I don't get why only MacOS is failing, since the low level code is pretty much the same, we just continue right after we left the nested event loop. I agree that we should find out the root cause why MacOS behaves different. > I don't know of any supported way to run macOS in a VM on a Windows host, but > perhaps others will be able to help. That was also my understanding the last time I checked. AFAIK, think there is no official way from Apple, but VirtualBox may offer that. Will check as soon as I have more time. - PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1962428672
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v2]
On Thu, 22 Feb 2024 23:13:26 GMT, Marius Hanl 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
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen
On Thu, 22 Feb 2024 23:18:46 GMT, Marius Hanl wrote: > maybe it's time to setup a VM on my Windows machine. Are there any > recommendations I may should follow for the setup? :-) I don't know of any supported way to run macOS in a VM on a Windows host, but perhaps others will be able to help. > Regarding the system tests, I checked the nested event loop ones on Windows > and they work for me. It worked for me on Windows, too (and on Linux in a headful test run). I would want to make sure that we know _why_ they still work on Windows and Linux and fail on Mac, but I only saw the failures on Mac. > I just pushed a change which may fixes this for Mac as well. The change is > also logical and is synchronized with the other changes I made to nested > event loops. Will ask my colleague as well to test my changes again + run the > most obvious (related) system tests on Mac. I can also fire off a headful test run this afternoon, and post results tomorrow. > I also will do a much bigger testing soon, I have an application that relies > heavily on nested event loops for blocking the UI while doing background work. The results of that testing would be good information. - PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1960552062
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen
On Thu, 22 Feb 2024 15:41:55 GMT, Kevin Rushforth wrote: > @Maran23 I see from the description that you tested this on mac. How did you > test it? Did you run with `-PFULL_TEST=true`? No, I checked the example in the ticket as well as the handling of dialogs in general on Mac. Unfortuantely, I always need to ask a colleague to test my JavaFX Builds on Mac, but maybe it's time to setup a VM on my Windows machine. Are there any recommendations I may should follow for the setup? :-) Regarding the system tests, I checked the nested event loop ones on Windows and they work for me. I just pushed a change which may fixes this for Mac as well. The change is also logical and is synchronized with the other changes I made to nested event loops. Will ask my colleague as well to test my changes again + run the most obvious (related) system tests on Mac. I also will do a much bigger testing soon, I have an application that relies heavily on nested event loops for blocking the UI while doing background work. - PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1960498210
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen [v2]
> 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 - Changes: - all: https://git.openjdk.org/jfx/pull/1324/files - new: https://git.openjdk.org/jfx/pull/1324/files/98a97b0a..cb8865e8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1324=01 - incr: https://webrevs.openjdk.org/?repo=jfx=1324=00-01 Stats: 294903 lines in 6630 files changed: 173936 ins; 83950 del; 37017 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
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen
On Tue, 9 Jan 2024 21:45:33 GMT, Marius Hanl 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
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen
On Tue, 9 Jan 2024 21:45:33 GMT, Marius Hanl 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 @Maran23 Please merge in the latest upstream master, since your branch is out of date. - PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1959579953
Re: RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen
On Tue, 9 Jan 2024 21:45:33 GMT, Marius Hanl 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 Hmm. There was a reason we added a return value to `_enterNestedEventLoop`, but I'll take a closer look. Have you run all of the headful tests? - PR Comment: https://git.openjdk.org/jfx/pull/1324#issuecomment-1959569754
RFR: JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen
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 - Commit messages: - JDK-8285893: Hiding dialog and showing new one causes dialog to be frozen Changes: https://git.openjdk.org/jfx/pull/1324/files Webrev: https://webrevs.openjdk.org/?repo=jfx=1324=00 Issue: https://bugs.openjdk.org/browse/JDK-8285893 Stats: 164 lines in 13 files changed: 12 ins; 81 del; 71 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
