Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-7176957

ExecutorService submit method javaDoc enhancement

    Details

    • Type: Enhancement
    • Status: Open
    • Priority: P4
    • Resolution: Unresolved
    • Affects Version/s: 7
    • Fix Version/s: None
    • Component/s: core-libs
    • Labels:

      Description

      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.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                Unassigned
                Reporter:
                webbuggrp Webbug Group
              • Votes:
                0 Vote for this issue
                Watchers:
                0 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Imported:
                  Indexed: