On Thu, 3 Sep 2026 15:38:47 GMT, Kevin Rushforth <[email protected]> wrote:
>> Why do you print this exception at all? This might end up in application >> logs, but without any possibility for remediation. It's also not relevant if >> there's nothing you can do about it. >> >> Thinking even further, what catching the exception does is: it tells callers >> that everything was successful. But we've still retained a native timer >> object, and `isRunning()` continues to report `true`. Restarting the timer >> remains possible, where the native timer would be silently overwritten. >> >> Instead of changing the semantics of every Windows timer, we could instead >> throw the exception because a failed cancellation is an actual invariant >> failure, and move the recovery to where we need it to be resilient. This >> could be `QuantumToolkit.exit()` or `dispose()`. > >> Why do you print this exception at all? This might end up in application >> logs, but without any possibility for remediation. It's also not relevant if >> there's nothing you can do about it. > > We often log exceptions in cases like this for diagnostic purposes. My > reading of the Microsoft docs is that this should never happen as long as we > use a valid timer object. If it does unexpectedly happen, logging it seems > better than being silent about it. > >> Thinking even further, what catching the exception does is: it tells callers >> that everything was successful. But we've still retained a native timer >> object, and `isRunning()` continues to report `true`. Restarting the timer >> remains possible, where the native timer would be silently overwritten. > > Yes, and if we actually used the timer restart, this would be a more serious > problem, but it still needs to be handled. > >> Instead of changing the semantics of every Windows timer, we could instead >> throw the exception because a failed cancellation is an actual invariant >> failure, and move the recovery to where we need it to be resilient. This >> could be `QuantumToolkit.exit()` or `dispose()`. > > Yes, this is the approach I was planning to address in the follow-up. But > maybe it is worth addressing in this PR. It would not be an intrusive change, > so I'll take a look at it. I pushed a new commit to address this. Any remaining concerns around Timer lifecycle can be dealt with in a follow-up issue. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2290#discussion_r3962891541
