[concurrency-interest] Unreported RuntimeException if Future.get()is never invoked

David Holmes dcholmes at optusnet.com.au
Thu Dec 21 21:05:46 EST 2006


A number of people get tripped up over FutureTask not allowing there to be
any uncaught exceptions. This is a problem when they are used certain ways.
The basic issue is that anytime you invoke an API that will execute a "task"
for you then you also would like to be able to specify what to do if the
task throws any exceptions.

The most flexible approach would allow you to specify a different handler
per task:

  public void execute(Runnable r, UncaughtExceptionHander ueh)

where the UEH would have a handleException method that takes the Runnable
and the Throwable, similar to the existing Thread based UEH mechanism.

I'm not aware of any framework that goes to this much trouble. It is a
little cumbersome and unfortunately it is probably too late to redefine UEH
to use Runnable rather than Thread.

So a second approach is to have the execution framework provide an "uncaught
exception handling" mechanism. This is less flexible as it tends to provide
a handler per Executor and that only works well when the executor is
executing homogenous work.

The third approach is to recognize that given the first approach is too
cumbersome, and the second is too inflexible, then there isn't a general
solution for the framework so you throw the onus back onto the definer of
the tasks themselves. After all these should be the people that know how
they would like uncaught exceptions handled. So you can do this yourself
with a custom Runnable. Or we did this with FutureTask.

The problem then is that once we have adopted this
task-based-uncaught-exception-handling policy, if you actually want a
per-Executor policy then you either need to ensure you only get tasks that
don't handle the exceptions themselves (ie don't use FutureTask - not an
option in Java 5), or you have to provide work-arounds in the executor to
extract the exception information from the tasks - as per your proposal.

Personally I think the best solution is the cumbersome one: have the ability
to define a per-task UEH, combined with a default per-executor UEH, similar
to how we do the Thread UEH. But that would still require an alternative
FutureTask implementation to not handle the exceptions itself. And of course
there is nothing you can do about the end tasks catching everything
themselves anyway.

Cheers,
David Holmes

> -----Original Message-----
> From: concurrency-interest-bounces at cs.oswego.edu
> [mailto:concurrency-interest-bounces at cs.oswego.edu]On Behalf Of Kevin
> Condon
> Sent: Friday, 22 December 2006 12:11 AM
> To: concurrency-interest at cs.oswego.edu
> Subject: [concurrency-interest] Unreported RuntimeException if
> Future.get()is never invoked
>
>
> If you use ExecutorService.submit() to start a task, but you never
> invoke the returned Future's get(), then you'll never see any evidence
> of RuntimeExceptions thrown during task execution.  Undetected
> exceptions are bad and it was pretty difficult to figure out what was
> happening in my own code.  I'm currently correcting several places in
> our codebase where this vulnerability exists and thought it might be a
> public service to point out to others the pit I fell into.  :)
>
> There are a variety of reasons that you want a Future but may never
> invoke get().  One situation where this could happen is a concurrent
> task with a cancel option, so you only need the Future for the cancel.
>  Here is a skeleton example:
>
> public class CancellableTask implements Runnable {
>   private final ExecutorService exec;
>   private volatile Future<?> future = null;
>
>   public CancellableTask(ExecutorService exec) {
>     this.exec = exec;
>   }
>
>   public void execute() {
>     // example ignores race on future for concurrent task executions
>     future = exec.submit(this);
>   }
>
>   public void cancel() {
>     if (future != null) {
>       future.cancel(true);
>     }
>   }
>
>   public void run() {
>     boolean error = false;
>     // work to be done ...
>     if (error) {
>       // this will not be logged!!!!
>       throw IllegalStateException("Secret exception");
>     }
>   }
> }
>
> A co-worker and I, JCiP in hand, discovered that the solution to this
> is to extend FutureTask overriding the done() hook to check/log the
> exception and then use exec.execute(futureTask) instead of
> submit(this).  To do this, change the execute() method:
>
>   public void execute() {
>     FutureTask<?> futureTask = new FutureTask<?>(this, null) {
>       protected void done() {
>         try {
>           get();
>         } catch (ExecutionException ex) {
>           ex.getCause().printStackTrace();  // logging or handling of
> your choice
>         } catch (CancellationException ex) {
>           // ignore unless you want to log task cancellation here
>         } catch (InterruptedException ex) {
>           // ignore; we're done, so get() won't block or be interrupted
>         }
>       }
>     };
>     future = futureTask;
>     exec.execute(futureTask);
>   }
>
> Hope this helps someone to avoid stumbling into this, because without
> knowing this can happen the problem can go entirely unnoticed.
>
> Regards,
> Kevin
> _______________________________________________
> Concurrency-interest mailing list
> Concurrency-interest at altair.cs.oswego.edu
> http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest



More information about the Concurrency-interest mailing list