Skip to content

ControlBusCommandRegistry registers beans for destruction lifecycle unintentionally #9890

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
t-ondrej opened this issue Mar 13, 2025 · 1 comment

Comments

@t-ondrej
Copy link

In what version(s) of Spring Integration are you seeing this issue?

Since 6.4

Describe the bug

The ControlBusCommandRegistry implements DestructionAwareBeanPostProcessor but does not override requiresDestruction, which defaults to true. As a result, beans that are not meant to be processed during the destroy lifecycle are being registered for the procedure (AbstractBeanFactory#registerDisposableBeanIfNecessary). We are receiving loads of SimpleThreadScope does not support destruction callbacks.Consider using RequestScope in a web environment. logs.

To Reproduce

Run the tests in https://github.com/t-ondrej/control-bus-command-registry-bug repo

Expected behavior

The ControlBusCommandRegistry implements requiresDestruction with proper post processing eligibility check.

@t-ondrej t-ondrej added status: waiting-for-triage The issue need to be evaluated and its future decided type: bug labels Mar 13, 2025
@artembilan
Copy link
Member

Yeah... I thought that check is per DestructionAwareBeanPostProcessor , but apparently that is for the whole BeanFactory:

	/**
	 * Check whether the given bean has destruction-aware post-processors applying to it.
	 * @param bean the bean instance
	 * @param postProcessors the post-processor candidates
	 */
	public static boolean hasApplicableProcessors(Object bean, List<DestructionAwareBeanPostProcessor> postProcessors) {
		if (!CollectionUtils.isEmpty(postProcessors)) {
			for (DestructionAwareBeanPostProcessor processor : postProcessors) {
				if (processor.requiresDestruction(bean)) {
					return true;
				}
			}
		}
		return false;
	}

So, yes, the ControlBusCommandRegistry has to implement this requiresDestruction() to return true only for those beans processes by this registry.

@artembilan artembilan added this to the 6.5.0-M3 milestone Mar 13, 2025
@artembilan artembilan added for: backport-to-6.4.x in: core and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Mar 13, 2025
spring-builds pushed a commit that referenced this issue Mar 13, 2025
Fixes: #9890
Issue link: #9890

The `DestructionAwareBeanPostProcessor.requiresDestruction()` defaults to `true`.
As a result, beans that are not meant to be processed during the destroy lifecycle
are being registered for the `AbstractBeanFactory#registerDisposableBeanIfNecessary`.
for example, the `SimpleThreadScope` does not support destruction callbacks.

In other words, the `DestructionAwareBeanPostProcessor.requiresDestruction()` contract
is not merely for `DestructionAwareBeanPostProcessor` logic, but also has an effect
on the whole `BeanFactory`

* Implement `requiresDestruction()` in the `ControlBusCommandRegistry`
for same filter as `registerControlBusCommands()` does right now.
* Optimize behaviour using `MergedAnnotations` for the class instead of
`AnnotationUtils.findAnnotation()` twice, which creates the mentioned `MergedAnnotations` internally.

(cherry picked from commit 74cf59b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants