Overriding ThreadPoolExecutor afterExecute method - any cons?

Question

Pros of hook methods:

beforeExecute(Thread, Runnable) and afterExecute(Runnable, Throwable)

beforeExecute(Thread, Runnable) and afterExecute(Runnable, Throwable) methods that are called before and after execution of each task. These can be used to manipulate the execution environment; for example, reinitializing ThreadLocals, gathering statistics, or adding log entries

I am using Custom ThreadPoolExecutor to handle uncaught exceptions. I can add try{} catch{} blocks in Runnable and Callable but assume a scenario where you can't force developer to add these blocks in relevant Runnable and Callable tasks.

This CustomThreadPoolExecutor , overrides afterExecute() method in ThreadPoolExecutor as below ( I have assigned variable b value to Zero to simulate arithmetic exception.

import java.util.concurrent.*;
import java.util.*;

class CustomThreadPoolExecutor extends ThreadPoolExecutor {

   public CustomThreadPoolExecutor() { 
       super(1,10,60,TimeUnit.SECONDS,new ArrayBlockingQueue<Runnable>(1000));
   }

   protected void afterExecute(Runnable r, Throwable t) {
     super.afterExecute(r, t);
     if (t == null && r instanceof Future<?>) {
       try {
         Object result = ((Future<?>) r).get();
         System.out.println(result);
       } catch (CancellationException ce) {
           t = ce;
       } catch (ExecutionException ee) {
           t = ee.getCause();
       } catch (InterruptedException ie) {
           Thread.currentThread().interrupt(); // ignore/reset
       }
     }
     if (t != null)
       t.printStackTrace();
   }
 }


public class CustomThreadPoolExecutorDemo{

    public static void main(String args[]){
        System.out.println("creating service");
        //ExecutorService service = Executors.newFixedThreadPool(10);
        CustomThreadPoolExecutor service = new CustomThreadPoolExecutor();
        service.submit(new Runnable(){
                 public void run(){
                    int a=4, b = 0;
                    System.out.println("a and b="+a+":"+b);
                    System.out.println("a/b:"+(a/b));
                    System.out.println("Thread Name in Runnable after divide by zero:"+Thread.currentThread().getName());
                 }
            });
        service.shutdown();
    }
}

Since submit() hides exception at framework, I have overridden afterExecute() method to catch Exception.

In this method, I added blocking call with below statement

 Object result = ((Future<?>) r).get();

Currently I have 10 threads with queue capacity as 1000. Assume that my Runnable takes 5 seconds to complete.

By overriding afterExecute() method, am I incurring any performance overhead OR any cons with this approach?

Answer 1

No, your blocking call wouldn't bring an overhead, because task is already completed its execution and has status >= NORMAL as you can see in void runWorker(Worker w)

beforeExecute(wt, task);
Throwable thrown = null;
try {
    task.run();
} catch (RuntimeException x) {
    thrown = x; throw x;
} catch (Error x) {
    thrown = x; throw x;
} catch (Throwable x) {
    thrown = x; throw new Error(x);
} finally {
    afterExecute(task, thrown);
}

Answer 2

Better solution, hold on to the Future returned from submit() and then you can handle the exception in your main thread instead of hacking the executor to print it out for you.

Another alternative would be to use a common base Runnable which implements the exception handling that you desire, e.g.:

public abstract class BaseRunnable implements Runnable {
  public final run() {
    try {
      runImpl();
    } catch(Throwable t) {
      t.printStackTrace();
    }
  }
  protected abstract runImpl() throws Exception;
}