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).

-------------

Commit messages:
 - 8389783: Crash in JavaFX native glass toolkit during Timer shutdown

Changes: https://git.openjdk.org/jfx/pull/2290/files
  Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=2290&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8389783
  Stats: 240 lines in 4 files changed: 230 ins; 2 del; 8 mod
  Patch: https://git.openjdk.org/jfx/pull/2290.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/2290/head:pull/2290

PR: https://git.openjdk.org/jfx/pull/2290

Reply via email to