WebSocketMessagingAutoConfiguration causes Executor bean to be registered multiple times under different names

[emopti-jrufer]

Bug Description
WebSocketMessagingAutoConfiguration is using an existing registered Executor bean as the output from bean factory methods AbstractMessageBrokerConfiguration#clientInboundChannelExecutor and AbstractMessageBrokerConfiguration#clientOutboundChannelExecutor which are annotated with @Bean.

This causes the existing registered executor bean to go through the standard bean lifecycle interfaces multiple times. In addition the AbstractMessageBrokerConfiguration is also modifying the bean depending on the executor implementation.

In the case of ThreadPoolTaskExecutor it has several lifecycle interfaces such as BeanNameAware, InitializingBean, ApplicationContextAware, DisposableBean and ApplicationListener which get called for each registration.

In short registering a single instances of a bean multiple times can lead to unexpected issues and would assume most beans are not designed with this intent. AbstractMessageBrokerConfiguration is expecting separate unregistered Executor instances that it will register as beans.

I tripped over this while trying to debug an issue preventing the our application from shutting down. I first noticed the bean name on the applicationTaskExecutor - ThreadPoolTaskExecutor had a bean name of clientOutboundChannelExecutor set inside the instance. Then I figured out the reason why our app was not shutting down was the destroy method of the executor was being called multiple times due to being registered multiple times in the context.

Replication
https://github.com/emopti-jrufer/spring-boot/tree/executer_bug
See org.springframework.boot.websocket.autoconfigure.servlet.WebSocketMessagingAutoConfigurationTests#bug

Workaround
As a workaround I excluded WebSocketMessagingAutoConfiguration and had to configure the message converters in a separate WebSocketMessageBrokerConfigurer.

[jyt6640]

I’d like to work on this.

I can start by adding a failing test that verifies the existing applicationTaskExecutor instance is not reused as clientInboundChannelExecutor / clientOutboundChannelExecutor.

Does that sound like the right direction?

[dongyikuan919]

I looked at the interaction between Boot's auto-configuration and Spring Framework's message broker configuration, and the root cause seems to be the object identity of the executor passed into the channel registration.

Boot currently does this in WebSocketMessagingAutoConfiguration.SpringBootWebSocketMessageBrokerConfigurer:

registration.executor(this.executor);

Then Spring Framework's AbstractMessageBrokerConfiguration exposes the channel executors as @Bean methods:

@Bean
public Executor clientInboundChannelExecutor() {
    ChannelRegistration registration = getClientInboundChannelRegistration();
    Executor executor = getExecutor(registration, "clientInboundChannel-", this::defaultExecutor);
    ...
    return executor;
}

and the same pattern is used for clientOutboundChannelExecutor. If the registration contains the existing Boot applicationTaskExecutor, those @Bean methods return the already-managed executor instance again under new bean names. That explains the repeated BeanNameAware / initialization / destruction behavior on the same ThreadPoolTaskExecutor instance.

For a fix, I would avoid passing a managed executor bean directly to ChannelRegistration unless Framework changes its contract. A lightweight delegating executor that does not implement the Spring lifecycle callbacks would preserve the reuse of the underlying applicationTaskExecutor for execution while preventing the same managed bean instance from being registered again as clientInboundChannelExecutor and clientOutboundChannelExecutor.

For the test, besides asserting that the existing applicationTaskExecutor is not reused as those two beans, it may be worth asserting object identity explicitly:

assertThat(context.getBean("clientInboundChannelExecutor"))
    .isNotSameAs(context.getBean(TaskExecutionAutoConfiguration.APPLICATION_TASK_EXECUTOR_BEAN_NAME));
assertThat(context.getBean("clientOutboundChannelExecutor"))
    .isNotSameAs(context.getBean(TaskExecutionAutoConfiguration.APPLICATION_TASK_EXECUTOR_BEAN_NAME));

That would catch the lifecycle problem directly rather than only observing shutdown symptoms.

[wilkinsona]

Thanks for the report. In the first instance at least, I think this needs to be investigated by the Spring Framework team. The contract for configuring the client channels and their executors makes no mention that they will be exposed as beans. If this is intentional then a fix only in Boot will not address the problem for other users of the API. Some updates to javadoc may help to avoid the problem, or perhaps Framework itself can avoid the problem by doing some wrapping.

[jyt6640]

@wilkinsona Thanks for the clarification.

I agree this likely needs to be addressed at the Framework level first, especially since the current contract does not indicate that the provided executor may later be exposed as a bean.

I'll hold off on a PR for now and continue investigating possible approaches on the Framework side (wrapping, lifecycle implications, bean registration expectations, etc.).

If a Framework-side change ends up being the preferred direction, I'd be happy to help prepare and contribute a candidate fix.