On Thu, 27 Mar 2025 15:21:09 GMT, Martin Fox <m...@openjdk.org> wrote:
>> There is an undocumented limit on nesting calls to CFRunLoopRun (or the >> equivalent wrapper NSRunLoop methods). When the limit is hit the OS >> terminates the Java app. The situation arises when a JavaFX app creates too >> many nested event loops from within Platform.runLater runnables. >> >> This PR doesn't change the limit (which is 250+ nested loops) but it does >> throw an exception just before the limit is reached so a JavaFX developer >> will get a useful Java stack trace instead of an OS crash log. >> >> On the Mac the nested event loop has two stages: first we ask the run loop >> to run, then we pull an event out and process it. A Platform.runLater >> runnable is executed in the first stage so if the runnable starts a new >> nested event loop the system will re-enter CFRunLoopRun. The same isn't true >> if an input event handler starts a new nested event loop; at that point >> we're in stage two and are past the call to CFRunLoopRun. > > Martin Fox has updated the pull request incrementally with two additional > commits since the last revision: > > - Removed unnecessary import > - The max nested event loop constant is no longer public The implementation looks good. I left one question / suggestion on the type of Exception throws. I think the suggestion made by @mstr2 to recommend calling `canStartNestedLoop` if the app wants to know whether or not this call will succeed is a good one. I think we should update the docs for `Stage::showAndWait` documenting the same exception, since the implementation of that method also spins up a nested event loop. Since this fix has been expanded to all platforms, let's remove `[macos]` from the title of the JBS issue and PR. modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 305: > 303: * other than the JavaFX Application Thread. > 304: * > 305: * @throws RuntimeException if this call would exceed the maximum I wonder if there is a more specific subclass of `RuntimeException` that we could throw? Maybe `IllegalStateException`, which seems the closest in meaning and has the advantage that we already throw it in other cases. ------------- PR Review: https://git.openjdk.org/jfx/pull/1741#pullrequestreview-2722832212 PR Comment: https://git.openjdk.org/jfx/pull/1741#issuecomment-2758937412 PR Review Comment: https://git.openjdk.org/jfx/pull/1741#discussion_r2017254207
