future.cancel does not work

Question

I have a nice and compact code, which does not work as I expected.

public class Test {

    public static void main(String[] args) {

        Runnable r = new Runnable() {
            @Override
            public void run() {
                try {
                    for (;;) {

                    }
                } finally {
                    System.out.println("FINALLY");
                }

            }
        };

        ExecutorService executor = Executors.newSingleThreadExecutor();

        Future<?> future = executor.submit(r);
        try {
            future.get(3, TimeUnit.SECONDS);
        } catch (TimeoutException e) {
            boolean c = future.cancel(true);
            System.out.println("Timeout " + c);
        } catch (InterruptedException | ExecutionException e) {
            System.out.println("interrupted");
        }
        System.out.println("END");

    }

}

The output is :

Timeout true

END

Question: Why does not terminate the future.cancel(true) method the called Runnable? After the program wrote the "END" to the output, the "r" Runnable is still running.

are you getting this output also? System.out.println("Timeout " + c); — Bart Hofma, Jan 20 '15 at 10:42
possible duplicate of [Future.cancel() method is not working](http://stackoverflow.com/questions/13623445/future-cancel-method-is-not-working) — pihentagy, Jan 20 '15 at 12:36

score 18 · Accepted Answer · edited Jan 05 '22 at 21:47

18

The problem is that your Runnable is not interruptible: task interruption is a collaborative process in Java and the cancelled code needs to check regularly if it's been cancelled, otherwise it won't respond to the interruption.

You can amend you code as follows and it should work as expected:

Runnable r = new Runnable() {
    @Override public void run() {
        try {
            while (!Thread.currentThread().isInterrupted()) {}
        } finally {
            System.out.println("FINALLY");
        }
    }
};

edited Jan 05 '22 at 21:47

Laurel

5,965
14
31
57

answered Jan 20 '15 at 10:53

assylias

321,522
82
660
783

2

The programm still wont stop then, cause the executor needs to be shutdown: executor.shutdown(); -> at the end of the main. Dunno if this is even wanted to be done at that point tho. – Wagner Michael Jan 20 '15 at 10:58
@maffelbaffel You are right - I only addressed the OP's question. – assylias Jan 20 '15 at 11:23
2

just mentioned it because people thend to forget the shutdown ;) - like all the other close()'s – Wagner Michael Jan 20 '15 at 19:53

score 2 · Answer 2 · edited Jan 20 '15 at 16:27

This is always a little bit misleading: The ExceutorService or even the underlying thread scheduler do not know anything about what the Runnable is doing. In your case they don't know that there is a unconditional loop.

All these methods (cancel, done, ...) are related to manage Threads in the Executor structure. cancel cancels the thread from the point of view of the Executor service.

The programmer must test if the Runnable was canceled and must terminate the run() method.

So in your case (if I remember well) something like this:

public class Test {

public static void main(String[] args) {

    FutureTask r = new FutureTask () {
        @Override
        public void run() {
            try {
                for (;!isCancelled();) {

                }
            } finally {
                System.out.println("FINALLY");
            }

        }
    };

    ExecutorService executor = Executors.newSingleThreadExecutor();

    Future<?> future = executor.submit(r);
    try {
        future.get(3, TimeUnit.SECONDS);
    } catch (TimeoutException e) {
        boolean c = future.cancel(true);
        System.out.println("Timeout " + c);
    } catch (InterruptedException | ExecutionException e) {
        System.out.println("interrupted");
    }
    System.out.println("END");

}

}

This does not compile, and even if you fix it (so that a `Runnable` is passed to `FutureTask` constructor), methods of the `FutureTask` object are not callable within the body of the `Runnable::run()` method. — Joe Coder, Jun 19 '19 at 10:44
As @JoeCoder mentioned the code above doesn't compile, I suggest replacing `FutureTask` declaration with the following `FutureTask r = new FutureTask<>(() -> { try { for (; !Thread.currentThread().isInterrupted(); ) {} } finally { System.out.println("FINALLY"); } }, "The result");` — Eugene T, Oct 31 '20 at 14:05

score 1 · Answer 3 · answered Jan 20 '15 at 10:53

When you cancel a Future whose Runnable has already begun, the interrupt method is called on the Thread that is running the Runnable. But that won't necessarily stop the thread. Indeed, if it's stuck in a tight loop, like the one you've got here, the Thread won't stop. In this case, the interrupt method just sets a flag called the "interrupt status", which tells the thread to stop when it can.

See the Javadoc for the interrupt method of Thread

score 1 · Answer 4 · answered Jun 17 '20 at 22:27

Future.cancel() will cancel any queued task or will call Thread.interrupt() on your thread if already running.

You need to interrupt your code

It's your code's responsibility is to be ready for any interruptions. I'd go so far to say that whenever you have a long running task, that you insert some interrupt ready code like this:

while (... something long...) {

     ... do something long

     if (Thread.interrupted()) {
         ... stop doing what I'm doing...
     }
}

How to stop what I'm doing?

You have several options:

If your you are in Runnable.run() just return or break out of the loop and finish the method.
You may be in some other method deep in the code. It may make sense at that point for that method to throw InterruptedException so you would just do that (leaving the flag cleared).
But maybe deep in your code it doesn't make sense to throw InterruptedException. In that case you should throw some other exception, but before that mark your thread interrupted again so the code that catches knows that an interrupt was in progress. Here's an example:

private void someMethodDeepDown() {
    while (.. long running task .. ) {
          ... do lots of work ...

          if (Thread.interrupted()) {
             // oh no! an interrupt!
             Thread.currentThread().interrupt();
             throw new SomeOtherException();
          }
     }
}

Now the exception can propagate an either terminate the thread or be caught, but the receiving code hopefully notices that an interrupt is in progress.

future.cancel does not work

4 Answers4