JDK-8160037 : ExecutorService::shutdownNow may block invokeAll indefinitely
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util.concurrent
  • Affected Version: 8,9
  • Priority: P4
  • Status: Open
  • Resolution: Unresolved
  • OS: generic
  • CPU: generic
  • Submitted: 2016-06-20
  • Updated: 2022-05-10
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbd_majorUnresolved
Related Reports
Relates :  
Description
FULL PRODUCT VERSION :


A DESCRIPTION OF THE PROBLEM :
ExecutorService::invokeAll may block indefinitely, if shutdownNow was called and not all tasks have been started.

InvokeAll calls get for all FutureTasks, which are not done. After calling shutdownNow this will block forever as the FutureTask will never be executed. Obviously pending tasks are not interrupted.

From AbstractExecutorService:

    public <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks)
        throws InterruptedException {
        if (tasks == null)
            throw new NullPointerException();
        ArrayList<Future<T>> futures = new ArrayList<Future<T>>(tasks.size());
        boolean done = false;
        try {
            for (Callable<T> t : tasks) {
                RunnableFuture<T> f = newTaskFor(t);
                futures.add(f);
                execute(f);
            }
            for (int i = 0, size = futures.size(); i < size; i++) {
                Future<T> f = futures.get(i);
                if (!f.isDone()) {
                    try {
                        f.get();              <--------------------------------------- Blocks here
                    } catch (CancellationException ignore) {
                    } catch (ExecutionException ignore) {
                    }
                }
            }
            done = true;
            return futures;
        } finally {
            if (!done)
                for (int i = 0, size = futures.size(); i < size; i++)
                    futures.get(i).cancel(true);
        }
    }


EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
In the testcase below the message "invokeAll returned" should be printed
ACTUAL -
The message "invokeAll returned" is not printed. executorInvokerThread hangs.

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
public class ThreadTest {

	@Test
	public void executorServiceTest() {
		ExecutorService executor = Executors.newFixedThreadPool(2);
		
		List<Callable<Void>> tasks = new ArrayList<>();
		tasks.add((( Callable<Void>) () -> { System.out.println("callable 1"); Thread.sleep(500); return null; }) );
		tasks.add((( Callable<Void>) () -> { System.out.println("callable 2"); Thread.sleep(500); return null; }) );
		tasks.add((( Callable<Void>) () -> { System.out.println("callable 3"); Thread.sleep(500); return null; }) );
		tasks.add((( Callable<Void>) () -> { System.out.println("callable 4"); Thread.sleep(500); return null; }) );
		tasks.add((( Callable<Void>) () -> { System.out.println("callable 5"); Thread.sleep(500); return null; }) );
		tasks.add((( Callable<Void>) () -> { System.out.println("callable 6"); Thread.sleep(500); return null; }) );

		Thread executorInvokerThread = new Thread( () -> {
			try {
				executor.invokeAll(tasks);
			} catch (Exception e) {
				e.printStackTrace();
			}
			
			System.out.println("invokeAll returned");
		});
		executorInvokerThread.start();
		
		try {
			Thread.sleep(800);
		} catch (InterruptedException e1) {
			e1.printStackTrace();
		}
		
		
		System.out.println("ShutdownNow");
		executor.shutdownNow();
		try {
			executor.awaitTermination(2, TimeUnit.MINUTES);
		} catch (InterruptedException e) {
			e.printStackTrace();
		}
		
		System.out.println("Shutdown complete");
		try {
			executorInvokerThread.join(50000);
		} catch (InterruptedException e) {
			e.printStackTrace();
		}
	}

}

---------- END SOURCE ----------


Comments
perhaps there's something we can add to the docs
24-06-2016

Stuart makes a good point. There is a mismatch between the Runnable-oriented Executor interface and the Future-oriented ExecutorService interface. With ThreadPoolExecutor, if you execute() a Runnable, that Runnable is enqueued and returned by shutdownNow. But if you submit() a Runnable, a FutureTask is enqueued and returned by shutdownNow. So in fact users can in practice do for (Runnable r : pool.shutdownNow()) { if (r instanceof Future) ((Future)r).cancel(false); } but I don't think we guarantee that they can do this. Is there some way we can improve the docs?
24-06-2016

Unfortunately shutdownNow returns a List<Runnable> so it's not clear (to me at least) how the caller of shutdownNow can cancel those tasks. Perhaps a note in the documentation could recommend something, though I'm not sure what the recommendation would be.
24-06-2016

I agree with Martin. The behavior matches the specifications -- shutdownNow returns the list of tasks, that the user should cancel if appropriate (that's why they are returned).
24-06-2016

The observed behavior seems like a natural result of the specs. shutdownNow returns a list of never-started tasks, while invokeAll waits for all tasks to be completed, so it seems natural for it to hang if the never-started task list is simply discarded. It is possible for the caller of shutdownNow to cancel all returned tasks, releasing the caller of invokeAll. I can't think of anything we could do to improve matters.
24-06-2016

Attached test case executed on : JDK 8 - Fail JDK 8u92 - Fail JDK 9ea b122 - fail Output: JUnit version 4.4 .callable 1 callable 2 callable 3 callable 4 ShutdownNow Shutdown complete Time: 50.849 OK (1 test)
22-06-2016