-
Enhancement
-
Resolution: Unresolved
-
P4
-
None
-
7
-
x86
-
windows_xp
A DESCRIPTION OF THE REQUEST :
There is a subtle non-functional bug that is easy to fall into related to the exception handling behaviour of the java.util.concurrent.ExecutorService#submit vs execute methods. The java-doc for the submit method should be enhanced to warn about this issue. The issue is this:
If you use submit instead of execute to queue a Runnable with an ExecutorService, and if you don’t retain/inspect the result (Future) then even if you have created the ExecutorService with a ThreadFactory that sets up an UncaughtExcetionHandler, then you will lose any exceptions thrown by that runnable.
This can be a very easy mistake to make, the code (apart from exception handling) is identical so in the normal use-case would work just fine, only when things go wrong, and your exceptions are silently lost would you have a problem that might be hard to diagnose.
JUSTIFICATION :
Warn users of a potential bug that they might make through incorrect usage of the ExecutorService API.
EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
javaDoc for the java.util.concurrent.ExecutorService#submit method should include a paragraph similar to this:
"<b>WARNING</B> care should be taken not to use this method for 'fire-and-forget' task submission. In the case where you do not make use of the resultant Future, then the {@link #execute(Runnable)} should probably be used instead. This is because these two methods have a subtle difference in behaviour when it comes to exception handling/propagation. The submit method will catch all exceptions thrown by the runnable, and will inject the exception into the returned Future. When calling Future.get() any injected exception is then re-thrown. However; the execute method allows any exceptions thrown by the Runnable to bubble up and (if present) will be passed to the thread's UncaughtExceptionHandler. Since it is common practice to configure a thread's UncaughtExceptionHandler to (at the very least) ensure that any uncaught exceptions are logged, use of the submit method instead of the execute method could mean that any uncaught exceptions are <b>NOT</b> logged. Instead these uncaught exceptions would have been injected into the unassigned/unused Future returned by the submit method. "
---------- BEGIN SOURCE ----------
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.ThreadFactory;
import java.util.logging.Level;
import java.util.logging.Logger;
public class ExecutorServiceSubmitMethodUsageExample {
public static void main(String[] args) throws Exception {
final Logger logger = Logger.getLogger("example");
ExecutorService es = Executors.newSingleThreadExecutor(new ThreadFactory() {
public Thread newThread(final Runnable r) {
Thread t = new Thread(r);
t.setUncaughtExceptionHandler(new Thread.UncaughtExceptionHandler() {
public void uncaughtException(Thread t, Throwable e) {
logger.log(Level.WARNING, "Uncaught exception in thread " + t.getName(), e);
}
});
return t;
}
});
es.execute(new Runnable() {
public void run() {
throw new RuntimeException("This exception will be logged");
}
});
//Incorrect usage of submit
es.submit(new Runnable() {
public void run() {
throw new RuntimeException("This exception will be lost"); //!!!!!!!!
}
});
//Correct usage of submit
final Future<?> fut = es.submit(new Runnable() {
public void run() {
throw new RuntimeException("This exception will be thrown as a wrapped ExecutionExecution from Future.get");
}
});
try {
fut.get();
} catch (ExecutionException ee) {
logger.log(Level.WARNING, "Unhandled Exception", ee);
}
}
}
---------- END SOURCE ----------
CUSTOMER SUBMITTED WORKAROUND :
Don't think this issue can easily be avoided, it is a coding/API usage issue that the javaDoc should warn about.
There is a subtle non-functional bug that is easy to fall into related to the exception handling behaviour of the java.util.concurrent.ExecutorService#submit vs execute methods. The java-doc for the submit method should be enhanced to warn about this issue. The issue is this:
If you use submit instead of execute to queue a Runnable with an ExecutorService, and if you don’t retain/inspect the result (Future) then even if you have created the ExecutorService with a ThreadFactory that sets up an UncaughtExcetionHandler, then you will lose any exceptions thrown by that runnable.
This can be a very easy mistake to make, the code (apart from exception handling) is identical so in the normal use-case would work just fine, only when things go wrong, and your exceptions are silently lost would you have a problem that might be hard to diagnose.
JUSTIFICATION :
Warn users of a potential bug that they might make through incorrect usage of the ExecutorService API.
EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
javaDoc for the java.util.concurrent.ExecutorService#submit method should include a paragraph similar to this:
"<b>WARNING</B> care should be taken not to use this method for 'fire-and-forget' task submission. In the case where you do not make use of the resultant Future, then the {@link #execute(Runnable)} should probably be used instead. This is because these two methods have a subtle difference in behaviour when it comes to exception handling/propagation. The submit method will catch all exceptions thrown by the runnable, and will inject the exception into the returned Future. When calling Future.get() any injected exception is then re-thrown. However; the execute method allows any exceptions thrown by the Runnable to bubble up and (if present) will be passed to the thread's UncaughtExceptionHandler. Since it is common practice to configure a thread's UncaughtExceptionHandler to (at the very least) ensure that any uncaught exceptions are logged, use of the submit method instead of the execute method could mean that any uncaught exceptions are <b>NOT</b> logged. Instead these uncaught exceptions would have been injected into the unassigned/unused Future returned by the submit method. "
---------- BEGIN SOURCE ----------
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.ThreadFactory;
import java.util.logging.Level;
import java.util.logging.Logger;
public class ExecutorServiceSubmitMethodUsageExample {
public static void main(String[] args) throws Exception {
final Logger logger = Logger.getLogger("example");
ExecutorService es = Executors.newSingleThreadExecutor(new ThreadFactory() {
public Thread newThread(final Runnable r) {
Thread t = new Thread(r);
t.setUncaughtExceptionHandler(new Thread.UncaughtExceptionHandler() {
public void uncaughtException(Thread t, Throwable e) {
logger.log(Level.WARNING, "Uncaught exception in thread " + t.getName(), e);
}
});
return t;
}
});
es.execute(new Runnable() {
public void run() {
throw new RuntimeException("This exception will be logged");
}
});
//Incorrect usage of submit
es.submit(new Runnable() {
public void run() {
throw new RuntimeException("This exception will be lost"); //!!!!!!!!
}
});
//Correct usage of submit
final Future<?> fut = es.submit(new Runnable() {
public void run() {
throw new RuntimeException("This exception will be thrown as a wrapped ExecutionExecution from Future.get");
}
});
try {
fut.get();
} catch (ExecutionException ee) {
logger.log(Level.WARNING, "Unhandled Exception", ee);
}
}
}
---------- END SOURCE ----------
CUSTOMER SUBMITTED WORKAROUND :
Don't think this issue can easily be avoided, it is a coding/API usage issue that the javaDoc should warn about.
- relates to
-
JDK-7178766 Executor Service exceptions propagate to uncaught exception handler
-
- Closed
-
-
JDK-8224858 ThreadPoolExecutor/Callable silently ignores all uncaught exception
-
- Closed
-