Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8176254

ScheduledThreadPoolExecutor periodic tasks not cancelled if running at shutdown

    Details

    • Type: Bug
    • Status: In Progress
    • Priority: P3
    • Resolution: Unresolved
    • Affects Version/s: 8
    • Fix Version/s: 10
    • Component/s: core-libs
    • Labels:
      None

      Description

      It appears to be the intent that Futures for all thread pool tasks are complete after awaitTermination returns. But for periodic tasks that are actively executing when shutdown is called asynchronously, they are not cancelled and Future.get never returns.

      import static java.util.concurrent.TimeUnit.SECONDS;
      import java.util.concurrent.*;

      public class STPEShutdown {
          public static void main(String[] args) throws Throwable {
              final int n = 4;
              final Future<?>[] futures = new Future<?>[n];
              final ScheduledThreadPoolExecutor pool
                  = new ScheduledThreadPoolExecutor(n/2);
              final CountDownLatch running = new CountDownLatch(n/2);
              final CountDownLatch proceed = new CountDownLatch(1);
              final Runnable task = () -> {
                  try {
                      running.countDown();
                      proceed.await();
                  } catch (InterruptedException t) { throw new Error(t); }
              };
              for (int i = 0; i < n; i++)
                  futures[i] = pool.scheduleWithFixedDelay(task, 0, 1, SECONDS);
              running.await();
              pool.shutdown();
              proceed.countDown();
              if (!pool.awaitTermination(10, SECONDS))
                  throw new Error("timed out");
              for (Future<?> future : futures) {
                  System.err.printf("isDone=%s isCancelled=%s%n",
                                    future.isDone(),
                                    future.isCancelled());
              }
          }
      }

      prints:
      isDone=false isCancelled=false
      isDone=false isCancelled=false
      isDone=true isCancelled=true
      isDone=true isCancelled=true

      The spec is not entirely clear on what should happen. It seems that the spec and the implementation should be improved to match the apparent common sense intent. As far as possible, it should be guaranteed that futures are complete on executor termination, that cancellation of the future inhibits subsequent execution of the task, and that abnormal termination of a periodic task not only inhibits subsequent executions but is also reflected in the Future's result.

        Activity

        Hide
        dholmes David Holmes added a comment -
        How does that relate to your example? As per my earlier comments the example behaves as I would expect.

        If future invocations are cancelled then the Future's state reflects that of the last invocation. That was either in-progress when cancellation happened - in which case it has completed normally - or it was pending and so it has been cancelled.
        Show
        dholmes David Holmes added a comment - How does that relate to your example? As per my earlier comments the example behaves as I would expect. If future invocations are cancelled then the Future's state reflects that of the last invocation. That was either in-progress when cancellation happened - in which case it has completed normally - or it was pending and so it has been cancelled.
        Hide
        martin Martin Buchholz added a comment -
        Periodic tasks never set the future's normal result. get() can never return normally.

        Currently if shutdown happens during periodic task execution, then future.get() will hang indefinitely.
        Show
        martin Martin Buchholz added a comment - Periodic tasks never set the future's normal result. get() can never return normally. Currently if shutdown happens during periodic task execution, then future.get() will hang indefinitely.
        Hide
        dholmes David Holmes added a comment -
        But in your example for the Future's that report "not cancelled" get() would return? I'm still trying to tie this back to the example.

        And this just highlights even more a need for a specification as to how a Future representing a series of tasks behaves.
        Show
        dholmes David Holmes added a comment - But in your example for the Future's that report "not cancelled" get() would return? I'm still trying to tie this back to the example. And this just highlights even more a need for a specification as to how a Future representing a series of tasks behaves.
        Hide
        dholmes David Holmes added a comment -
        With the updated example (thanks) I would expect:

        isDone=false isCancelled=false

        to be:

        isDone=true isCancelled=false

        using the "Future represents the current invocation" view.

        However, if the Future is intended to represent the series then the series can never be done (get() can't return normally), but can be cancelled.

        I see now that the way the Future is acting is inconsistent.
        Show
        dholmes David Holmes added a comment - With the updated example (thanks) I would expect: isDone=false isCancelled=false to be: isDone=true isCancelled=false using the "Future represents the current invocation" view. However, if the Future is intended to represent the series then the series can never be done (get() can't return normally), but can be cancelled. I see now that the way the Future is acting is inconsistent.
        Hide
        martin Martin Buchholz added a comment -
        Somehow I didn't notice the existing spec

        http://download.java.net/java/jdk9/docs/api/java/util/concurrent/ScheduledExecutorService.html#scheduleAtFixedRate-java.lang.Runnable-long-long-java.util.concurrent.TimeUnit-

        """The executor terminates, also resulting in task cancellation."""

        which makes it clearer the historic behavior is "just a bug".
        Show
        martin Martin Buchholz added a comment - Somehow I didn't notice the existing spec http://download.java.net/java/jdk9/docs/api/java/util/concurrent/ScheduledExecutorService.html#scheduleAtFixedRate-java.lang.Runnable-long-long-java.util.concurrent.TimeUnit- """The executor terminates, also resulting in task cancellation.""" which makes it clearer the historic behavior is "just a bug".

          People

          • Assignee:
            martin Martin Buchholz
            Reporter:
            martin Martin Buchholz
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated: