Document exception handling limitations in TaskDecorator implementations (specifically for ThreadPoolTaskExecutor#submit)

[archiecobbs]

Affects: demonstrated against 5.2.6.RELEASE

ThreadPoolTaskExecutor.setTaskDecorator() allows you to, for example, catch and log any exceptions thrown by tasks submitted to the executor. In my application this functionality is very important, because otherwise bugs that cause exceptions would be completely swallowed and lurk indefinitely, unnoticed.

The bug is that this works if you use ThreadPoolTaskExecutor.execute() to submit your task, but it doesn't work if you use ThreadPoolTaskExecutor.submit() to submit your task.

Basically, when invoking any ThreadPoolTaskExecutor method that returns a Future, there is extra wrapping involved that catches and swallows thrown exceptions before they can reach the configured TaskDecorator. This is a reversed wrapping order from that one would expect.

Here's a program that demonstrates the problem:

import org.springframework.core.task.TaskDecorator;
import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor;

public class ThreadPoolTaskExecutorBug {

    public static void main(String[] args) throws Exception {

        // Setup executor
        ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
        executor.setTaskDecorator(action -> () -> {
            try {
                System.out.println("invoking " + action + ".run()");
                action.run();
                System.out.println("successful return from " + action + ".run()");
            } catch (Throwable t) {
                System.out.println("caught exception from " + action + ".run(): " + t.getMessage());
            }
        });
        executor.afterPropertiesSet();

        System.out.println();
        System.out.println("TEST #1");
        executor.execute(new Action(1));
        Thread.sleep(500);

        System.out.println();
        System.out.println("TEST #2");
        executor.submit(new Action(2));
        Thread.sleep(500);

        System.out.println();
        executor.shutdown();
    }

    public static class Action implements Runnable {

        private final int id;

        public Action(int id) {
            this.id = id;
        }

        @Override
        public void run() {
            System.out.println(this + ": run() invoked");
            System.out.println(this + ": run() throwing exception");
            throw new RuntimeException("exception thrown by " + this);
        }

        @Override
        public String toString() {
            return "Action#" + this.id;
        }
    }
}

Here's the output:

Jun 10, 2020 10:32:18 AM org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor initialize
INFO: Initializing ExecutorService

TEST #1
invoking Action#1.run()
Action#1: run() invoked
Action#1: run() throwing exception
caught exception from Action#1.run(): exception thrown by Action#1

TEST #2
invoking java.util.concurrent.FutureTask@62f7c498.run()
Action#2: run() invoked
Action#2: run() throwing exception
successful return from java.util.concurrent.FutureTask@62f7c498.run()

Jun 10, 2020 10:32:19 AM org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor shutdown
INFO: Shutting down ExecutorService

Note that under TEST #2 the action throws an exception, just like in TEST #1, but the configured TaskDecorator never gets a chance to catch and log the exception.

[jhoeller]

This seems to be a common problem with the JDK's ThreadPoolExecutor that we're building on there: see the note in its afterExecute javadoc: https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ThreadPoolExecutor.html#afterExecute-java.lang.Runnable-java.lang.Throwable-

ThreadPoolExecutor.before/afterExecute is essentially a variant of our TaskDecorator, we suffer from the same limitations. Since TaskDecorator does not imply exception handling directly, we cannot manually force Future exceptions there. I'm afraid there is not much we can do about it; this unfortunately will have to be handled within affected TaskDecorator implementations :-(

[archiecobbs]

Thanks for the quick triage. If it can't be fixed then perhaps at least some additional documentation could be added to clarify the semantics of TaskDecorator, in particular, that exceptions may not reach it.