ThreadPoolTaskExecutor does not allow for decorating Runnables [SPR-13930]

[spring-projects-issues]

Dave Syer opened SPR-13930 and commented

The fact that ThreadPoolTaskExecutor.initializeExecutor() is protected makes it a public extension point, and it returns an Executor, so you might expect that overriding it will result in the return value being used at run time. In fact it is not because the ThreadPoolTaskExecutor does (though its parent class) set a field called "executor" but then never uses it, preferring instead to use its own more specialized ThreadPoolExecutor.

---

Affects: 4.2.4

Referenced from: commits 25be5e0

[spring-projects-issues]

Dave Syer commented

I believe the getThreadPoolExecutor() method should return an ExecutorService and that the value should be the one returned from initializeExecutor(). Or maybe there's a better way to wrap the executor that I missed?

[spring-projects-issues]

Dave Syer commented

I was able to use reflection hacks to override the executor like this:

@Override
public void configureClientInboundChannel(ChannelRegistration registration) {
     registration.taskExecutor(new ThreadPoolTaskExecutor() {
          @Override
          protected ExecutorService initializeExecutor(ThreadFactory threadFactory,
                        RejectedExecutionHandler rejectedExecutionHandler) {
               BlockingQueue<Runnable> queue = createQueue(Integer.MAX_VALUE);
               ThreadPoolExecutor executor  = new ThreadPoolExecutor(
                                 getCorePoolSize(), getMaxPoolSize(), getKeepAliveSeconds(), TimeUnit.SECONDS,
                                 queue, threadFactory, rejectedExecutionHandler) {
                        @Override
                        public void execute(Runnable command) {
                                 super.execute(wrap(runnable));
                        }
               };
               Field field = ReflectionUtils.findField(ThreadPoolTaskExecutor.class, "threadPoolExecutor");
               ReflectionUtils.makeAccessible(field);
               ReflectionUtils.setField(field, this, executor);
               return executor;
          }
     });
}

I'd be more than happy to try a workaround if anyone has an idea.

[spring-projects-issues]

Juergen Hoeller commented

Actually, for that kind of purpose, you might be better off using our ConcurrentTaskExecutor decorator, taking a custom ThreadPoolExecutor and simply wrapping it as a TaskExecutor. You won't get ThreadPoolTaskExecutor's runtime-configurable properties that way though.

Even if initializeExecutor allowed to fully override the executor handle, the outcome in your case would still be rather convoluted, wouldn't it. Instead, I'm inclined to just document initializeExecutor's limitations and rather introduce dedicated template methods for such overriding purposes: in your case, a decorateRunnable method on ThreadPoolTaskExecutor itself. After all, the JDK ThreadPoolExecutor has been designed for subclassing in very specific points only.

Juergen

[spring-projects-issues]

Juergen Hoeller commented

It might be even better to introduce a common strategy for this: TaskDecorator, supported by all our common executor variants. That'd be a simply lambda against a setter then, no need to subclass anything.

[spring-projects-issues]

Dave Syer commented

Sounds good to me, since that was what I needed to do. Ideally I need it before 4.3, but I guess I can go with reflection hacks until then.

[spring-projects-issues]

Juergen Hoeller commented

Just wondering: Would ConcurrentTaskExecutor as a decorator do the job for you at the moment? No need for reflections hacks there...

Juergen

[spring-projects-issues]

Juergen Hoeller commented

As suggested above, I've introduced a TaskDecorator interface, with support in all common TaskExecutor implementations.

Juergen

[spring-projects-issues]

Ruslan Al-Fakikh commented

@Juergen Hoeller, I encountered a need for the feature, but found that TaskDecorator support is not included in ThreadPoolTaskScheduler. Is there a reason for it and is there a workaround to decorate tasks in ThreadPoolTaskScheduler?

Ruslan

[spring-projects-issues]

Julian Ruppel commented

@Juergen Hoeller: I'm also wondering why there is no possibility to set TaskDecorator in ThreadPoolTaskScheduler