Future from MoreExecutors#listeningDecorator is never cancelled on executor service's termination

[GeorgiPetkov]

When using listening decorator on top of scheduled executor service and the service is terminated the future remains pending forever. I saw that you're using NeverSuccessfulListenableFutureTask which doesn't imply to be also never failing (in my case it should fail on cancel due to termination). Also the behavior is different than not using listening decorator and wrapping the future from scheduled executor service with the JdkFutureAdapters (in that case it works are expected).
Here are JUnit4 tests (one if which is failing) to make it more clear and notice the difference:

import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningScheduledExecutorService;
import org.junit.Test;

import java.util.concurrent.CancellationException;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeoutException;

import static com.google.common.util.concurrent.JdkFutureAdapters.listenInPoolThread;
import static com.google.common.util.concurrent.MoreExecutors.listeningDecorator;
import static java.util.concurrent.Executors.newScheduledThreadPool;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.junit.Assert.*;

public class ListenableFuturesTest {

    private static final Runnable DO_NOTHING = () -> {
    };

    @Test
    public void testScheduledExecutorServiceWithListeningDecorator() throws InterruptedException, TimeoutException, ExecutionException {
        ListeningScheduledExecutorService service = listeningDecorator(newScheduledThreadPool(1));
        ListenableFuture<?> future = service.scheduleAtFixedRate(DO_NOTHING, 0L, 1, SECONDS);

        assertFalse(future.isDone());

        service.shutdown();
        service.awaitTermination(5, SECONDS);
        service.shutdownNow();

        try {
            future.get(10, SECONDS);
            fail("Expected to throw an exception.");
        } catch (CancellationException ignored) {
        }

        assertTrue(future.isCancelled());
    }

    @Test
    public void testScheduledExecutorServiceWithJdkFutureAdapters() throws InterruptedException, TimeoutException, ExecutionException {
        ScheduledExecutorService service = newScheduledThreadPool(1);
        ListenableFuture<?> future = listenInPoolThread(service.scheduleAtFixedRate(DO_NOTHING, 0L, 1, SECONDS));

        assertFalse(future.isDone());

        service.shutdown();
        service.awaitTermination(5, SECONDS);
        service.shutdownNow();

        try {
            future.get(10, SECONDS);
            fail("Expected to throw an exception.");
        } catch (CancellationException ignored) {
        }

        assertTrue(future.isCancelled());
    }
}

[GeorgiPetkov]

BTW I'm using Java 8 with Guava version 28.0-jre.

[cpovirk]

(moved from #3554)

The spec for ScheduledExecutorService added this new requirement when moving from Java 8 to Java 9:

The sequence of task executions continues indefinitely until one of the following exceptional completions occur:
...

• The executor terminates, also resulting in task cancellation.

(Yes, the old spec says "Otherwise, the task will only terminate via cancellation or termination of the executor." But it's not clear that "task termination" (presumably meaning "Is the Runnable run again?") implies "Future cancellation." In particular, if termination implied cancellation, I would expect the spec to just say "Otherwise, the task will only terminate via cancellation [, such as by termination of the executor]." Anyway, it's mostly academic; the spec now says what is says, and I don't think the old spec forbids that behavior.)