vavr icon indicating copy to clipboard operation
vavr copied to clipboard

Future.isCancelled should return true when the executor is shutted down

Open thadumi opened this issue 5 years ago • 4 comments

Hello, I was playing around with Future's API and I'd like to open a discussion about the following use-case.

Reading the documentation of the Future#isCancelled I'd expect the following test should work, but it fails.

public void shouldReturnIsDone() {
        final ExecutorService executor = Executors.newSingleThreadExecutor();
        final Future<String> future = Future.of(executor, () -> {
            Try.run(() -> Thread.sleep(1000));
            return "Value never reach because will be cancelled";
        });
        executor.shutdownNow();
        assertThat(future.isCancelled()).isTrue();
}

Is this make on proposing or is it a bug? Taking a look at the source code a Future is cancelled only when it's the future itself call the interrupt method of the Thread.

thadumi avatar Apr 10 '19 16:04 thadumi

Thanks, that is an important finding! Yes, you are right - currently we only check, if a Future is actively interrupted by calling future.cancel(). We do not deal with the various cases a future might be internally interrupted by the runtime. This will be fixed. I need to re-implement FutureImpl for that purpose. Most probably I will fix this in 1.0.

danieldietrich avatar Apr 15 '19 12:04 danieldietrich

Note: In Scala only (write-granted) Promises are cancellable, rather than (readonly) Futures.

danieldietrich avatar Apr 15 '19 12:04 danieldietrich

Why are you saying that you have to re-implement FutureImp? I'd like to give a try to this issue. It should work using shutdown hooks. From Java Concurrency in Practice, chapter 7.4.1.:

In an orderly shutdown, the JVM first starts all registered shutdown hooks. Shutdown hooks are unstarted threads that are registered with Runtime.addShutdownHook. The JVM makes no guarantees on the order in which shutdown hooks are started. If any application threads (daemon or nondaemon) are still running at shutdown time, they continue to run concurrently with the shutdown process. When all shutdown hooks have completed, the JVM may choose to run finalizers if runFinalizersOnExit is true, and then halts. The JVM makes no attempt to stop or interrupt any application threads that are still running at shutdown time; they are abruptly terminated when the JVM eventually halts. If the shutdown hooks or finalizers don't complete, then the orderly shutdown process "hangs" and the JVM must be shut down abruptly. [...]

I never used the hooks but they could be usefull. I'll do some test to undestand if they are triggered also when an executor is shutting down.

thadumi avatar Apr 15 '19 15:04 thadumi

I think shutdown hooks are too specific and may not cover all use-cases. In VAVR 1.0 the FutureImpl will change nonetheless.

However, for v0.x you may go ahead!

Should a shutdown hook be created for each Future instance in order to provide access to the cancelled instance variable? That may lead to resource leaks when Futures are GC'ed...

danieldietrich avatar Apr 16 '19 09:04 danieldietrich