RFR: 8248381: Create a daemon thread for MonocleTimer

[Johan Vos]

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+jgneff at 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 at b1fe89
>   [Not completed, task = java.util.concurrent.Executors$RunnableAdapter at 1f85c96
>   [Wrapped task = com.sun.javafx.tk.quantum.QuantumToolkit$$Lambda$111/0x34563828 at 141859b]]
>   rejected from java.util.concurrent.ScheduledThreadPoolExecutor at 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