On Tue, 8 Sep 2026 22:40:44 GMT, Kevin Rushforth <[email protected]> wrote:
>> This PR fixes a bug in `Timer.stop()` on Windows that can happen if stop is >> called while a Timer callback is running. There are three related problems >> with the way the Timer is implemented on Windows: >> >> 1. The native timer kill operation is asynchronous: `timeSetEvent` does not >> include `TIME_KILL_SYNCHRONOUS` so `timeKillEvent` will not wait for a >> callback that might be executing concurrently. >> 2. The native `RunnableTimer::Stop()` method deletes the Timer object while >> it might still be in use running a callback. The deletion triggers a call to >> kill the timer from the Timer base class destructor, after the derived class >> destructor has run, meaning that the `runnable` member will be deleted >> before `timeKillEvent` is called. >> 3. The Java-side `Timer.pause()` method, which is a no-op on Windows, is >> synchronized on the Timer instance as is the `Timer.stop()` method. The >> `pause()` method is called from the Timer callback, which can lead to >> deadlock. >> >> This last problem might affect other platforms as well (e.g., it might >> contribute to a similar deadlock on macOS, tracked by >> [JDK-8238505](https://bugs.openjdk.org/browse/JDK-8238505)), but this fix is >> intentionally limited to Windows platform-specific code (except for the >> shared try/catch around `Timer.stop()` in toolkit shutdown). >> >> The fix is as follows: >> >> 1. Pass `TIME_KILL_SYNCHRONOUS` to `timeSetEvent`. >> 2. Move the `timeKillEvent` call from the Timer destructor to an explicit >> `cancel()` method. The destructor calls this method defensively (which >> should be a no-op in the typical case). >> 3. Call `cancel()` from `Stop()`, checking the return code. Delete the timer >> after a successful cancel operation; throw an exception if the timer cannot >> be canceled. This should never occur, but if it does, it is safer to leak >> the Timer object than free an object that might still be in use. >> 4. WinTimer.java - Override `pause()` and `resume()` to be unsynchronized >> no-op methods. >> 5. QuantumToolkit.java - Call `pulseTimer.stop()` in a try/catch from >> `QuantumToolkit.exit()` and `QuantumToolkit.dispose()` so that a >> cancellation failure doesn't abort toolkit shutdown; print the exception's >> stack trace if caught. >> >> I have included a test for this. To run just the new test: >> >> >> gradle sdk shims >> gradle --info -PTEST_ONLY=true -PFULL_TEST=true :systemTests:test --tests >> test.com.sun.glass.ui.WinTimerTest >> >> >> Without the fix, it will either crash (without generating an hs_err log) or >> fail with an Assert... > > Kevin Rushforth has updated the pull request incrementally with one > additional commit since the last revision: > > Moved try/catch to callers of Timer.stop() in QT shutdown; minor test > modifications lgtm ------------- Marked as reviewed by angorya (Reviewer). PR Review: https://git.openjdk.org/jfx/pull/2290#pullrequestreview-5147903872
