JDK-6267833 : Incorrect method signature ExecutorService.invokeAll()
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util.concurrent
  • Affected Version: 5.0
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: linux
  • CPU: x86
  • Submitted: 2005-05-10
  • Updated: 2017-05-16
  • Resolved: 2005-09-04
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.
JDK 6
6 b51Fixed
Description
FULL PRODUCT VERSION :
Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0-b64)

A DESCRIPTION OF THE PROBLEM :
The typed method signature for the invokeAll() call in ExecutorService is incorrect.  It should be declared:

<T> List<Future<T>> invokeAll(Collection<? extends Callable<T>>��tasks)

I have included a tiny test program which shows the compilation error 
that occurs because of the method declaration.  The method is declared to 
take a collection of exactly the type Callable, which is improper.  As the 
implementation of the invokeAll() does not need to add to the collection it 
is almost certainly not needed, but requires developers to reassign objects 
to expose the proper type when not building a new collection.

a better declaration would be:
<T> List<Future<T>> invokeAll(Collection<? extends Callable<T>>��tasks)

This change makes the class much more flexible, and the change should be 
made to the other similar methods in the class:
<T> List<Future<T>> invokeAll(Collection<? extends Callable<T>>��tasks,  
long��timeout,  TimeUnit��unit)
<T> T invokeAny(Collection<? extends Callable<T>>��tasks)
<T> T invokeAny(Collection<? extends Callable<T>>��tasks,  long��timeout,  
TimeUnit��unit)


REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
import java.util.*;
import java.util.concurrent.*;

public class Test implements Callable<Object> {

	public static void main(String[] args) {
		ExecutorService service = Executors.newSingleThreadExecutor();
		Test test = new Test();

		// This does not work because of the invokeAll() declaration.
		service.invokeAll(Collections.singleton(test));

		// This does but the assignment should not be necessary.
		Callable<Object> callable = test;
		service.invokeAll(Collections.singleton(callable));
	}

	public Object call() throws Exception {
		System.out.println("Called.");
	}
}

---------- END SOURCE ----------
###@###.### 2005-05-10 09:38:27 GMT

Comments
SUGGESTED FIX --- /u/martin/ws/mustang/src/share/classes/java/util/concurrent/AbstractExecutorService.java 2004-08-27 15:54:56.795363000 -0700 +++ /u/martin/ws/invoke/src/share/classes/java/util/concurrent/AbstractExecutorService.java 2005-08-09 20:53:16.362553000 -0700 @@ -49,7 +49,7 @@ /** * the main mechanics of invokeAny. */ - private <T> T doInvokeAny(Collection<Callable<T>> tasks, + private <T> T doInvokeAny(Collection<? extends Callable<T>> tasks, boolean timed, long nanos) throws InterruptedException, ExecutionException, TimeoutException { if (tasks == null) @@ -72,7 +72,7 @@ // result, we can throw the last exception we got. ExecutionException ee = null; long lastTime = (timed)? System.nanoTime() : 0; - Iterator<Callable<T>> it = tasks.iterator(); + Iterator<? extends Callable<T>> it = tasks.iterator(); // Start one task for sure; the rest incrementally futures.add(ecs.submit(it.next())); @@ -124,7 +124,7 @@ } } - public <T> T invokeAny(Collection<Callable<T>> tasks) + public <T> T invokeAny(Collection<? extends Callable<T>> tasks) throws InterruptedException, ExecutionException { try { return doInvokeAny(tasks, false, 0); @@ -134,13 +134,13 @@ } } - public <T> T invokeAny(Collection<Callable<T>> tasks, + public <T> T invokeAny(Collection<? extends Callable<T>> tasks, long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException { return doInvokeAny(tasks, true, unit.toNanos(timeout)); } - public <T> List<Future<T>> invokeAll(Collection<Callable<T>> tasks) + public <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks) throws InterruptedException { if (tasks == null) throw new NullPointerException(); @@ -170,7 +170,7 @@ } } - public <T> List<Future<T>> invokeAll(Collection<Callable<T>> tasks, + public <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks, long timeout, TimeUnit unit) throws InterruptedException { if (tasks == null || unit == null) --- /u/martin/ws/mustang/src/share/classes/java/util/concurrent/ExecutorService.java 2004-08-27 15:54:58.574521000 -0700 +++ /u/martin/ws/invoke/src/share/classes/java/util/concurrent/ExecutorService.java 2005-08-09 20:39:09.702387000 -0700 @@ -211,7 +211,7 @@ * for execution */ - <T> List<Future<T>> invokeAll(Collection<Callable<T>> tasks) + <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks) throws InterruptedException; /** @@ -240,7 +240,7 @@ * @throws RejectedExecutionException if any task cannot be scheduled * for execution */ - <T> List<Future<T>> invokeAll(Collection<Callable<T>> tasks, + <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks, long timeout, TimeUnit unit) throws InterruptedException; @@ -261,7 +261,7 @@ * @throws RejectedExecutionException if tasks cannot be scheduled * for execution */ - <T> T invokeAny(Collection<Callable<T>> tasks) + <T> T invokeAny(Collection<? extends Callable<T>> tasks) throws InterruptedException, ExecutionException; /** @@ -285,7 +285,7 @@ * @throws RejectedExecutionException if tasks cannot be scheduled * for execution */ - <T> T invokeAny(Collection<Callable<T>> tasks, + <T> T invokeAny(Collection<? extends Callable<T>> tasks, long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException; --- /u/martin/ws/mustang/src/share/classes/java/util/concurrent/Executors.java 2005-08-08 19:38:10.524811000 -0700 +++ /u/martin/ws/invoke/src/share/classes/java/util/concurrent/Executors.java 2005-08-09 20:40:22.248065000 -0700 @@ -617,20 +617,20 @@ public <T> Future<T> submit(Runnable task, T result) { return e.submit(task, result); } - public <T> List<Future<T>> invokeAll(Collection<Callable<T>> tasks) + public <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks) throws InterruptedException { return e.invokeAll(tasks); } - public <T> List<Future<T>> invokeAll(Collection<Callable<T>> tasks, + public <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks, long timeout, TimeUnit unit) throws InterruptedException { return e.invokeAll(tasks, timeout, unit); } - public <T> T invokeAny(Collection<Callable<T>> tasks) + public <T> T invokeAny(Collection<? extends Callable<T>> tasks) throws InterruptedException, ExecutionException { return e.invokeAny(tasks); } - public <T> T invokeAny(Collection<Callable<T>> tasks, + public <T> T invokeAny(Collection<? extends Callable<T>> tasks, long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException { return e.invokeAny(tasks, timeout, unit);
10-08-2005

WORK AROUND For the particular case of the submitter's program, a slightly simpler workaround is possible, using the syntax Collections.<Callable<Object>>singleton(x) ----------------------------------------------------------------- import java.util.*; import java.util.concurrent.*; public class Bug implements Callable<Object> { public static void main(String[] args) throws Throwable { ExecutorService service = Executors.newSingleThreadExecutor(); Bug x = new Bug(); // This does not work because of the invokeAll() declaration //service.invokeAll(Collections.singleton(x)); // This workaround works but should not be necessary service.invokeAll(Collections.<Callable<Object>>singleton(x)); service.shutdown(); } public Object call() throws Exception { System.out.println("Called."); return null; } } -----------------------------------------------------------------
10-08-2005

EVALUATION The JSR-166 expert group alumni agrees that the change: - is binary compatible. - is source compatible for *users* of an ExecutorService - requires minor source code changes for the small set of developers who have implemented ExecutorService without inheriting the default implementations in AbstractExecutorService. The set of affected developers are developers creating sophisticated thread pool applications, putting them into the "concurrency rocket scientist" category. They will generally appreciate this change. The possible compiler error is trivial to fix in the source code. There is always a tradeoff if/when such a slightly incompatible change is worth making. Consensus is that this change is worth making now.
10-08-2005