ConcurrentModificationException for late-registered AbstractEndpoints

[proggler23]

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

6.4.2.RELEASE

Describe the bug

After executing a test, annotated with @SpringBootTest and @SpringIntegrationTest, we saw a ConcurrentModificationException happening in SpringIntegrationTestExecutionListener.

The underlying issue was, that some logic added a new integrationFlow late (after application started). With bad timings, this happened exactly during afterTestClass execution currently looping through all AutoStartupCandidates, resulting in the exception.

To Reproduce

Register a new bean, extending AbstractEndpoint with autostartup = true exactly on test shutdown.

Expected behavior

Probably only handle Autostartup candidates, that are received during the ApplicationContext initialization (stop adding Autostartup candidates after ApplicationStarted).

Sample

Sample project

[proggler23]

probably it's already sufficient to just return a copy of the list for MockIntegrationContext.getAutoStartupCandidates?

[artembilan]

I think your:

  override fun doStop() {
        integrationFlowContext.registration(

is still valid use-case and it has nothing to do with testing or application shutdown.
We indeed can stop some endpoint manually at runtime and have respective logic over there.
The fact that same stop() is used when we shutdown application, is totally different story and has to be mitigated in the target project, respectively.

Such a bean registration during application shutdown has to fails somehow, e.g. with a:

/**
 * Exception thrown in case of a bean being requested despite
 * bean creation currently not being allowed (for example, during
 * the shutdown phase of a bean factory).
 *
 * @author Juergen Hoeller
 * @since 2.0
 */
@SuppressWarnings("serial")
public class BeanCreationNotAllowedException extends BeanCreationException {

But again: that is different story.

I think we should rework MockIntegrationContext to deal only with endpoints registered before application context is fully initialized.
In other words: do not react to those provided at runtime, e.g. via the mentioned register() API from the IntegrationFlowContext.
That's first.

Another side of the problem if we really need that SpringIntegrationTestExecutionListener.afterTestClass().
Since we talk about the application context shutdown, so all beans will be stopped anyway.
Or maybe not if we don't use @DirtiesContext and even share same test configuration between different test classes. "Context caching" in terms of Spring Framework: https://docs.spring.io/spring-framework/reference/testing/testcontext-framework/ctx-management/caching.html.
Either way, it looks like we can extract those mockIntegrationContext.getAutoStartupCandidates() as a property and use it in that afterTestClass().

Feel free to contribute the fix: https://github.com/spring-projects/spring-integration/blob/main/CONTRIBUTING.adoc!

[proggler23]

Thanks for already analyzing the issue!

Failing at application shutdown would be great. but the SpringIntegrationTestExecutionListener stops the AutoStartupCandidates BEFORE any spring application shutdowns or corresponding events are happening (so basically in the "middle of nowhere" in the application lifecycle).

Regarding the rework of MockIntegrationContext, I will try to contribute, but the project schedule is tight. Let's see :)

Best regards!

[proggler23]

@artembilan , only listening for AbstractEndpoints before application context is fully initialized unfortunatelly is also not working 100%.

I did miss to add another usecase:
override fun doStart() { integrationFlowContext.registration(

This then fails in the prepareTestInstance instead of afterTestClass

I would like to contribute only a small change to MockIntegrationContext first: getAutoStartupCandidates should return a new ArrayList instead of the internal one. This would at least resolve the ConcurrentModificationException, while still not fully solving the underlying issue.

What do you think about it?

[proggler23]

#9879