The message from this sender included one or more files which could not be scanned for virus detection; do not open these files unless you are certain of the sender's intent.
---------------------------------------------------------------------- On Sun, 28 Jun 2020 01:56:26 GMT, John Neffenger <github.com+1413266+jgn...@openjdk.org> wrote: >>> OK, that seems fine then. I'll take a closer look and then finish my review. >> >> Actually, I think you may be right, though. Sorry for replying before >> looking into it. I now think the >> `ScheduledThreadPoolExecutor` should be shut down, but let me look into it a >> bit more this afternoon before your final >> review. Thanks! The new `ScheduledThreadPoolExecutor` is ~complicated~ >> flexible! 😄 > > I think the code in the `_stop` method is correct after all. > > The `MonocleTimer` class is written to allow for multiple calls to the pair > of `_start` and `_stop` methods (even > though I don't think that ever happens), and the static > `ScheduledThreadPoolExecutor`, named `scheduler`, is created > only once and reused on subsequent calls. Changing the `_stop` method to > call `task.cancel(true)` still leaves the > timer thread running, which prevents the JavaFX application from exiting when > the timer thread is a user thread. > Furthermore, whether it's a user or daemon thread, if the call to > `task.cancel(true)` happens to run exactly when the > periodic task is *in progress*, the `timerRunnable` lambda in > `QuantumToolkit` prints the stack trace when it catches > the `InterruptedException`. java.lang.InterruptedException: > at javafx.graphics/com.sun.javafx.tk.quantum.QuantumToolkit > .lambda$runToolkit$12(QuantumToolkit.java:345) > at java.base/java.util.concurrent.Executors$RunnableAdapter > .call(Executors.java:515) > at java.base/java.util.concurrent.FutureTask > .runAndReset(FutureTask.java:305) > at > java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask > .run(ScheduledThreadPoolExecutor.java:305) > at java.base/java.util.concurrent.ThreadPoolExecutor > .runWorker(ThreadPoolExecutor.java:1130) > at java.base/java.util.concurrent.ThreadPoolExecutor$Worker > .run(ThreadPoolExecutor.java:630) > at java.base/java.lang.Thread > .run(Thread.java:832) > > So the call to `task.cancel(false)` is correct. > > Changing the `_stop` method to shut down the `scheduler` will terminate the > associated thread, regardless of its daemon > status, but a subsequent call to `_start` will throw a > `RejectedExecutionException` when trying to schedule the timer > task: java.util.concurrent.RejectedExecutionException: > Task > java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask@b1fe89 > [Not completed, task = > java.util.concurrent.Executors$RunnableAdapter@1f85c96 > [Wrapped task = > com.sun.javafx.tk.quantum.QuantumToolkit$$Lambda$111/0x34563828@141859b]] > rejected from java.util.concurrent.ScheduledThreadPoolExecutor@55f462 > [Terminated, pool size = 0, active threads = 0, queued tasks = 0, completed > tasks = 0] > at java.base/java.util.concurrent.ThreadPoolExecutor$AbortPolicy > .rejectedExecution(ThreadPoolExecutor.java:2057) > at java.base/java.util.concurrent.ThreadPoolExecutor > .reject(ThreadPoolExecutor.java:827) > at java.base/java.util.concurrent.ScheduledThreadPoolExecutor > .delayedExecute(ScheduledThreadPoolExecutor.java:340) > at java.base/java.util.concurrent.ScheduledThreadPoolExecutor > .scheduleAtFixedRate(ScheduledThreadPoolExecutor.java:632) > at javafx.graphics/com.sun.glass.ui.monocle.MonocleTimer > ._start(MonocleTimer.java:64) > at javafx.graphics/com.sun.glass.ui.Timer > .start(Timer.java:96) > at javafx.graphics/com.sun.javafx.tk.quantum.QuantumToolkit > .runToolkit(QuantumToolkit.java:384) > at javafx.graphics/com.sun.javafx.tk.quantum.QuantumToolkit > .lambda$startup$10(QuantumToolkit.java:280) > at javafx.graphics/com.sun.glass.ui.Application > .lambda$run$1(Application.java:153) > at javafx.graphics/com.sun.glass.ui.monocle.RunnableProcessor > .runLoop(RunnableProcessor.java:92) > at javafx.graphics/com.sun.glass.ui.monocle.RunnableProcessor > .run(RunnableProcessor.java:51) > at java.base/java.lang.Thread > .run(Thread.java:832) > > So if we want `MonocleTimer` to reuse a single `ScheduledThreadPoolExecutor` > object, I think the only way to make sure > that its timer thread exits when the application exits is to set its daemon > status to `true`. While the PR should indeed fix the original issue, I'm unsure about the behavior of multiple invocations of start/stop rather than using the (nop) pause method. However, it seems this behavior is similar on other platforms, so I assume it is by design. ------------- PR: https://git.openjdk.java.net/jfx/pull/256
