ConcurrentModificationException for registerEvent in TransactionalEventListener

[arne-kroeger]

I have a DDD project where I expose a domain object which extends AbstractAggregateRoot:

public class Document extends AbstractAggregateRoot<Document> {

    @Id
    public String id;

    public Document() {
      this.registerEvent(new DocumentCreated(this));
    }

    public void readyToRun() {
        this.registerEvent(new DocumentReadyToRun(this.id));
    }

}

Additionally there is a domain event listener in the same domain:

@Component
public class DocumentEventListener {

    @ApplicationModuleListener
    public void handle(DocumentCreated event) {
        event.document().readyToRun();
    }

}

In the other domain there is an additional event listener:

@Component
public class OtherDomainEventListener {

    @ApplicationModuleListener
    public void handle(DocumentReadyToRun event) {
        doWhatEverNeeded(event.documentId());
    }

}

Generally it uses the spring mongo implementation as the storage layer. If the service is called the following exception is thrown:

java.util.ConcurrentModificationException: null
	at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1095) ~[na:na]
	at java.base/java.util.ArrayList$Itr.next(ArrayList.java:1049) ~[na:na]
	at java.base/java.util.Collections$UnmodifiableCollection$1.next(Collections.java:1078) ~[na:na]
	at org.springframework.data.repository.core.support.EventPublishingRepositoryProxyPostProcessor$EventPublishingMethod.publishEventsFrom(EventPublishingRepositoryProxyPostProcessor.java:199) ~[spring-data-commons-3.3.1.jar:3.3.1]
	at org.springframework.data.repository.core.support.EventPublishingRepositoryProxyPostProcessor$EventPublishingMethodInterceptor.invoke(EventPublishingRepositoryProxyPostProcessor.java:116) ~[spring-data-commons-3.3.1.jar:3.3.1]
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184) ~[spring-aop-6.1.10.jar:6.1.10]
	at org.springframework.data.mongodb.repository.support.CrudMethodMetadataPostProcessor$CrudMethodMetadataPopulatingMethodInterceptor.invoke(CrudMethodMetadataPostProcessor.java:158) ~[spring-data-mongodb-4.3.1.jar:4.3.1]
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184) ~[spring-aop-6.1.10.jar:6.1.10]
	at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:97) ~[spring-aop-6.1.10.jar:6.1.10]
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184) ~[spring-aop-6.1.10.jar:6.1.10]
	at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:223) ~[spring-aop-6.1.10.jar:6.1.10]
	at jdk.proxy3/jdk.proxy3.$Proxy120.save(Unknown Source) ~[na:na]
        ....

[christophstrobl]

It would be great if you could take the time to provide a complete minimal sample (something that we can unzip or git clone, build, and deploy) that reproduces the problem.

[arne-kroeger]

Sure, see here: https://github.com/arne-kroeger/spring-data-test-event-error

[mp911de]

The problem that happens here is that we obtain a collection of domain events. While processing domain events, the list of events is being modified.

We could create a local copy of the domain events collection, call clear domain events first and then publish events to avoid leaving events unprocessed and clearing these.

With clear-before-publish domain events would remain in the aggregate root waiting for the next processing round.

Paging @odrotbohm for additional thoughts.

[odrotbohm]

To capture a bit more architectural context: the scenario in place here is problematic, as it manipulates the list of events during event handling. This in turn is caused by the transactional setup being incomplete. Using MongoDB in transactional mode needs explicit setup. With that missing, the @ApplicationModuleListener is executed as a simple @EventListener. This is a bug in Spring Framework already reported and fixed for 6.2 in spring-projects/spring-framework#32319. This can be tested by upgrading the project to Boot 3.4 M1, in which the event listener would not be invoked at all. Furthermore, if the setup is corrected to properly use transactions, the collection manipulation would be deferred until post persistence operation completion and not manifest at all.

This brings us to the root of the problem: an event must not contain references to mutable objects and the listener mutating that object. Registering additional events expands the issue into that new event being lost, as it would end up in the collection after its processing has started. Ideally, the event would only contain the identifier of the entity and the listener would look up the instance, initiate a state transition and complete the unit of work through a call to the repository. This would then cause the new event published properly.

We can and should fix our collection processing to create a defensive copy for the iteration. What I am still unsure about is in how far we should still surface the issue of an event having been registered during the processing. There are essentially three options:

1. Recursively processing events – We could re-obtain the events collection from the aggregate after a round of processing and publish the newly added events until that process does not find additional events anymore. This might become problematic as event processing cycles can be triggered easily. Furthermore, it will essentially cause an event to be published that might not be published if the original event was rather emitted through ApplicationEventPublisher directly (as in DocumentEventListener.handle(DocumentCreated)). I don't think we would want to expand our convenience mechanism to fundamentally work differently than application event publication of the framework in general.
2. Log a warning – We could detect the additional event being registered and issue a warning notifying the user about the lost event and describe the solution to this (using an identifier, resolve the reference in the listener, trigger the state transition).
3. Throw an exception – Basically a stricter version of 2 but making sure the problem will not make it into production at all.

I think we could even go with 3 as this has never worked at all but would just surface in a not very obvious way. Still letting the approach fail but describing what's the problem in a better way sounds like the safest option to me.

Feedback welcome!