On Thu, 19 Sep 2024 23:15:34 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> This PR fixes a hang on exit which can happen if QuantumToolkit.dispose >> hangs when called from the QuantumToolkit shutdown hook. A shutdown hook >> should never run indefinitely, so the fix is to call dispose from a >> background thread and wait for up to 5 seconds for it to finish normally, >> and then throw an Error so that the application will be able to exit anyway. >> >> The QuantumToolkit registers a "Glass/Prism Shutdown Hook" that calls the >> QuantumToolkit.dispose method to gracefully shutdown the toolkit and >> renderer. If dispose hangs or deadlocks, this will prevent the application >> from exiting, requiring the process to be killed. This has been observed on >> macOS, due to [JDK-8238505](https://bugs.openjdk.org/browse/JDK-8238505), >> which is is an intermittent deadlock that occurs during shutdown. We ought >> to eventually fix that intermittent deadlock, but even in the presence of a >> deadlock or hang, we don't want the shutdown hook to hang. >> >> This can happen at any time, but is especially a problem during an automated >> test run, which will then hang until the Jenkins job is either killed or its >> global timer (currently set to 12 hours) kills the job. Even in that case, >> processes can be left lying around which can affect subsequent runs. >> >> I discovered this while testing >> [JDK-8328629](https://bugs.openjdk.org/browse/JDK-8328629), which adds a >> default Unit 5 timeout, and without fixing the deadlock, the timeout will be >> unable to abort the hanging test. >> >> ### Testing instructions >> >> I tested this in connection with PR #1575 / >> [JDK-8328629](https://bugs.openjdk.org/browse/JDK-8328629) using my >> [test-timeout-and-shutdown-hang](https://github.com/kevinrushforth/jfx/tree/test-timeout-and-shutdown-hang) >> branch. That branch has the fixes from both this PR and PR #1575 as well as >> tests that will hang indefinitely without the fixes and fail due to the >> expected timeout with the fixes. > > modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/QuantumToolkit.java > line 263: > >> 261: @Override public void run() { >> 262: // Run dispose in a background thread and wait for up to >> 263: // 5 seconds for it to finish. If it doesn't, then >> throw an > > is 5 seconds a good timeout value? > > could it be shorter? > is it possible for the normal shutdown on a real system take more than 5 > seconds? I think 5 seconds is a reasonable timeout value. I wouldn't want it to be any shorter nor too much longer. Normally, this is something that would take a few milliseconds. FWIW, we have a similar 5 second timeout in the (macOS-specific) initialization code that waits for app activation. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1574#discussion_r1768482493
