On Wed, 2 Sep 2026 20:15:43 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. > > 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 `stop()` to call `super.stop()` in a try/catch, > so an exception doesn't cause `Toolkit.exit()` to hang, and print the > exception's stack trace if caught. > 5. WinTimer.java - Override `pause()` and `resume()` to be unsynchronized > no-op methods. > > 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 AssertionError. With the fix, it passes. > > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). modules/javafx.graphics/src/main/java/com/sun/glass/ui/win/WinTimer.java line 73: > 71: super.stop(); > 72: } catch (IllegalStateException ex) { > 73: ex.printStackTrace(); we should consider using a standardized logging subsystem at some point modules/javafx.graphics/src/main/java/com/sun/glass/ui/win/WinTimer.java line 100: > 98: public void resume() { > 99: } > 100: extra newline tests/system/src/test/java/test/com/sun/glass/ui/WinTimerTest.java line 169: > 167: assertNull(throwable.get()); > 168: } > 169: extra newline ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2290#discussion_r3918331536 PR Review Comment: https://git.openjdk.org/jfx/pull/2290#discussion_r3918333059 PR Review Comment: https://git.openjdk.org/jfx/pull/2290#discussion_r3918347143
