Description
An event published within the handling of a transactional event listener with its phase set to BEFORE_COMMIT
will NOT be handled by another transactional event listener which is also assigned to the BEFORE_COMMIT
phase.
Expected
If a BEFORE_COMMIT
listener publishes an event listened to by another BEFORE_COMMIT
listener, that event listener should get called in the already running before commit phase.
sequenceDiagram;
participant Component
participant EventPublisher
participant Transaction
participant A as Listener A
participant B as Listener B
Component-->>+Transaction: start
Component-->>EventPublisher: publish event A;
EventPublisher-->>Transaction: defer listener A execution to commit;
Component-->>Transaction: commit
Transaction-->>A: handle event
A-->>EventPublisher: publish event B;
EventPublisher-->>Transaction: defer listener B execution to commit;
Transaction-->>B: handle event
Transaction-->>-Component: committed
Actual
If a BEFORE_COMMIT
listener publishes an event listened to by another BEFORE_COMMIT
listener, that event listener does not get called and the event is "lost".
sequenceDiagram;
participant Component
participant EventPublisher
participant Transaction
participant A as Listener A
participant B as Listener B
Component-->>+Transaction: start
Component-->>EventPublisher: publish event A;
EventPublisher-->>Transaction: defer listener A execution to commit;
Component-->>Transaction: commit
Transaction-->>A: handle event
A-->>EventPublisher: publish event B;
EventPublisher-->>Transaction: defer listener B execution to commit;
Transaction-->>-Component: committed
Affected versions
Tested with Spring Boot 3.5 and 3.4. Both show the same behavior.
Reproduction
Small Spring Boot test for reproduction.
Maven POM:
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>3.5.0</version>
<relativePath/> <!-- lookup parent from repository -->
</parent>
<groupId>de.nimelrian</groupId>
<artifactId>before-commit-reproduction</artifactId>
<version>0.0.1-SNAPSHOT</version>
<properties>
<java.version>21</java.version>
</properties>
<dependencies>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-jdbc</artifactId>
</dependency>
<dependency>
<groupId>com.h2database</groupId>
<artifactId>h2</artifactId>
<scope>runtime</scope>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-test</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
<build>
<plugins>
<plugin>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-maven-plugin</artifactId>
</plugin>
</plugins>
</build>
</project>
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureJdbc;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.context.event.ApplicationEvents;
import org.springframework.test.context.event.RecordApplicationEvents;
import org.springframework.transaction.event.TransactionPhase;
import org.springframework.transaction.event.TransactionalEventListener;
import org.springframework.transaction.support.TransactionTemplate;
import static org.assertj.core.api.Assertions.assertThat;
@SpringBootTest
@AutoConfigureJdbc
@RecordApplicationEvents
class DemoApplicationTests {
private static final Logger log = LoggerFactory.getLogger(DemoApplicationTests.class);
@Test
void while_committing_additionally_published_events_should_be_handled(
@Autowired TransactionTemplate txTemplate,
@Autowired ApplicationEventPublisher applicationEventPublisher,
@Autowired ApplicationEvents applicationEvents
) {
txTemplate.executeWithoutResult(tx -> {
log.info("Publishing EventA");
applicationEventPublisher.publishEvent(new EventA());
log.info("Committing transaction");
});
assertThat(applicationEvents.stream(EventA.class)).as("EventA should have been published").hasSize(1);
assertThat(applicationEvents.stream(EventB.class)).as("EventB should have been published by listener for EventA").hasSize(1);
assertThat(applicationEvents.stream(EventC.class)).as("EventC should have been published by listener for EventB").hasSize(1); // This fails
}
@Configuration
static class TestContextConfiguration {
private final ApplicationEventPublisher applicationEventPublisher;
TestContextConfiguration(ApplicationEventPublisher applicationEventPublisher) {
this.applicationEventPublisher = applicationEventPublisher;
}
@TransactionalEventListener(phase = TransactionPhase.BEFORE_COMMIT)
void handleEventA(EventA eventA) {
log.info("handleEventA");
applicationEventPublisher.publishEvent(new EventB());
}
@TransactionalEventListener(phase = TransactionPhase.BEFORE_COMMIT)
void handleEventB(EventB eventB) {
log.info("handleEventB");
applicationEventPublisher.publishEvent(new EventC());
}
}
record EventA() {
}
record EventB() {
}
record EventC() {
}
}
Log output:
INFO [demo] [ main] com.zaxxer.hikari.HikariDataSource : HikariPool-1 - Starting...
INFO [demo] [ main] com.zaxxer.hikari.pool.HikariPool : HikariPool-1 - Added connection conn0: url=jdbc:h2:mem:9e2b7744-82f9-47aa-a3b3-5b866c6a8e06 user=SA
INFO [demo] [ main] com.zaxxer.hikari.HikariDataSource : HikariPool-1 - Start completed.
INFO [demo] [ main] de.nimelrian.demo.DemoApplicationTests : Publishing EventA
INFO [demo] [ main] de.nimelrian.demo.DemoApplicationTests : Committing transaction
INFO [demo] [ main] de.nimelrian.demo.DemoApplicationTests : handleEventA
java.lang.AssertionError: [EventC should have been published by listener for EventB]
Expected size: 1 but was: 0 in:
[]
at de.nimelrian.demo.DemoApplicationTests.while_committing_additionally_published_events_should_be_handled(DemoApplicationTests.java:40)
at java.base/java.lang.reflect.Method.invoke(Method.java:580)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
Comment From: Nimelrian
Note: This probably occurs due to TransactionSynchronizationManager.getSynchronizations
returning an UnmodifiableList of the listeners for events (in form of synchronizations with a beforeCommit
handler) which have been published at the start of the beforeCommit triggers: https://github.com/spring-projects/spring-framework/blob/v6.2.8/spring-tx/src/main/java/org/springframework/transaction/support/TransactionSynchronizationUtils.java#L125-L129
public static void triggerBeforeCommit(boolean readOnly) {
for (TransactionSynchronization synchronization : TransactionSynchronizationManager.getSynchronizations()) {
synchronization.beforeCommit(readOnly);
}
}
If events get published during the execution of listeners they get registered as new synchronizations, but the foreach loop obviously doesn't take these into account since the list with synchronizations is only created once at the start of the loop. Maybe this could instead make use of a queue from which synchronizations are popped and executed (with new synchronizations being emplaced at the back) until none are left...
Comment From: jhoeller
I'm afraid the current behavior is by design: Transaction synchronizations are ordered, without an expectation that new instances come in during a particular phase which would imply a different overall order. From that perspective, a particular listener's callback in the BEFORE_COMMIT phase is not meant to be able to register another synchronization that still runs in the BEFORE_COMMIT phase.
For a transactional event published, fundamentally the same semantics apply. If at all, we could only revisit those semantics for transactional events specifically, not for synchronizations themselves. However, that would be at the expense of consistent semantics between the two.
Could you rather design the follow-up events as regular @EventListener
methods which would execute immediately, always triggered by the initial @TransactionalEventListener
in the BEFORE_COMMIT phase?
Comment From: Nimelrian
For a transactional event published, fundamentally the same semantics apply. If at all, we could only revisit those semantics for transactional events specifically, not for synchronizations themselves. However, that would be at the expense of consistent semantics between the two.
Are the consistent semantics between synchronizations and events part of the public API of transactional events? If not, the technical implementation of events via synchronizations should not restrict the use of transactional events. At least in my opinion.
Right now each listener is registered as its own synchronization. Would it be possible to delegate all listeners to a single synchronization which would also be able to deal with new events and reordering (disregarding already executed listeners) during listener execution?
Could you rather design the follow-up events as regular
@EventListener
methods which would execute immediately, always triggered by the initial@TransactionalEventListener
in the BEFORE_COMMIT phase?
To get stuff done we already removed transactional listeners entirely and migrated to regular listeners. This comes with the disadvantage of instant execution and thus huge stack traces, especially since we use domain events to publish events once objects are passed to create/update/delete methods of repositories. Not only do the stacks grow very quickly, but they now also jump from the persistence implementation of one domain module to event listeners of another module:
domainARepository.update(DomainAObject)
domainBListener.handleDomainAObjectChanged(DomainAObjectChanged)
[domainB Logic]
domainBRepository.update(DomainBObject)
domainCListener.handleDomainBObjectChanged(DomainBObjectChanged)
[...]
So some method of event deferral would be nice to have. My expectation of an "ideal" mechanism as shown in the original post should still allow deferral of listener execution.