TransactionScoped beans are destroyed after the JTA transaction is commited/rolled back

[mkouba]

Describe the bug

This functionality was added in https://github.com/quarkusio/quarkus/pull/14053. A JTA Synchronization is registered and the CDI context is destroyed during the afterCompletion() callback.

However, our docs say the opposite: https://quarkus.io/guides/transaction#transaction-scope

The JTA spec states that "Any Synchronization.afterCompletion methods will be invoked in an undefined context.". But it does not explicitly define when exactly @TransactionScoped beans are destroyed.

We should either update the docs or change the implementation.

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

[mkouba]

CC @yrodiere beacause of https://github.com/quarkusio/quarkus/pull/28870 ;-)

[yrodiere]

Do I understand correctly that your problem is with @PreDestroy being called after commit/rollback?

This does sound counter-intuitive... But I guess it makes sense from CDI's point of view.

It would seem that I got confused by this test:

https://github.com/quarkusio/quarkus/blob/eed0241eaf6dfdd1f148e14dd153edf7404ac2d1/integration-tests/narayana-jta/src/main/java/io/quarkus/narayana/jta/TransactionBeanWithEvents.java#L138-L144

It says we expect an "active" transaction, but doesn't check that the status of the transaction is active... and I guess it's COMMITTED/ROLLEDBACK instead :/

[Ladicek]

The only way how @PreDestroy callbacks could be called before commit/rollback is to destroy the transaction CDI context as the first thing of the commit/rollback process, before we actually start committing or rolling back anything. However, the JTA specification says that the transaction CDI context is active even when the transaction status is committing and rolling back, so my opinion is that our documentation is just incorrect.

[mkouba]

The only way how @PreDestroy callbacks could be called before commit/rollback is to destroy the transaction CDI context as the first thing of the commit/rollback process, before we actually start committing or rolling back anything. However, the JTA specification says that the transaction CDI context is active even when the transaction status is committing and rolling back, so my opinion is that our documentation is just incorrect.

And it seems that Naryana JTA does the same in a CDI Full environment: https://github.com/jbosstm/narayana/blob/main/ArjunaJTA/cdi/classes/com/arjuna/ats/jta/cdi/TransactionScopeCleanup.java#L45-L51

[mkouba]

It says we expect an "active" transaction, but doesn't check that the status of the transaction is active... and I guess it's COMMITTED/ROLLEDBACK instead :/

Also this tests context lifecycle events which is a bit different.

[yrodiere]

The only way how @PreDestroy callbacks could be called before commit/rollback is to destroy the transaction CDI context as the first thing of the commit/rollback process, before we actually start committing or rolling back anything. However, the JTA specification says that the transaction CDI context is active even when the transaction status is committing and rolling back, so my opinion is that our documentation is just incorrect.

And it seems that Naryana JTA does the same in a CDI Full environment: https://github.com/jbosstm/narayana/blob/main/ArjunaJTA/cdi/classes/com/arjuna/ats/jta/cdi/TransactionScopeCleanup.java#L45-L51

I'm afraid this makes the @PreDestroy callback pretty much useless for many use cases... and also pretty confusing IMO: "you can execute code after the transaction gets committed by annotating the transaction-scoped bean with @**Pre**Destroy"...

Anyway, not the first time I disagree with the Transactions spec (who thought it was a good idea to have checked exceptions not cause a rollback in the interceptor by default? :x ), and I agree my opinion doesn't matter much in this case.

+1 to update the documentation to reflect what's standard and implemented, whatever that is.

[mkouba]

Well, a @PreDestroy should be used to clean up some "local" resources; "local" in the terms of bean internals. In general, it's not recommended to work with injected beans inside a @PreDestroy callback unless they live in a wider scope (e.g. @ApplicationScoped in this particular case).

But I agree that it's a bit confusing.

[Ladicek]

I'm afraid this makes the @PreDestroy callback pretty much useless for many use cases... and also pretty confusing IMO: "you can execute code after the transaction gets committed by annotating the transaction-scoped bean with @**Pre**Destroy"...

I'm no JTA expert, but I don't intuitively see why @TransactionScoped beans should be destroyed while the transaction still exists? Destroying the context after the transaction completes seems like a sensible choice to me.

It is also perfectly sensible to be able to get notified when a transaction is about to complete... which is possible using transactional observer methods: https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0.html#transactional_observer_methods I would recommend using those over lifecycle callbacks, if you're interested in observing the state of the transaction (as opposed to observing the state of the bean).

[yrodiere]

I'm afraid this makes the @PreDestroy callback pretty much useless for many use cases... and also pretty confusing IMO: "you can execute code after the transaction gets committed by annotating the transaction-scoped bean with @**Pre**Destroy"...

I'm no JTA expert, but I don't intuitively see why @TransactionScoped beans should be destroyed while the transaction still exists? Destroying the context after the transaction completes seems like a sensible choice to me.

It is also perfectly sensible to be able to get notified when a transaction is about to complete... which is possible using transactional observer methods: https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0.html#transactional_observer_methods I would recommend using those over lifecycle callbacks, if you're interested in observing the state of the transaction (as opposed to observing the state of the bean).

Fair enough. We do need an event to be fired in order to use @Observes(during = Transaction.BEFORE_COMPLETION) though... If I just want to run something before every transaction gets committed, is there an event type I can use?

[mkouba]

I'm afraid this makes the @PreDestroy callback pretty much useless for many use cases... and also pretty confusing IMO: "you can execute code after the transaction gets committed by annotating the transaction-scoped bean with @**Pre**Destroy"...

I'm no JTA expert, but I don't intuitively see why @TransactionScoped beans should be destroyed while the transaction still exists? Destroying the context after the transaction completes seems like a sensible choice to me. It is also perfectly sensible to be able to get notified when a transaction is about to complete... which is possible using transactional observer methods: https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0.html#transactional_observer_methods I would recommend using those over lifecycle callbacks, if you're interested in observing the state of the transaction (as opposed to observing the state of the bean).

Fair enough. We do need an event to be fired in order to use @Observes(during = Transaction.BEFORE_COMPLETION) though... If I just want to run something before every transaction gets committed, is there an event type I can use?

You can use the context lifecycle events, like in the test you mentioned above: void beforeDestroyed(@Observes @BeforeDestroyed(TransactionScoped.class) Object event).

[yrodiere]

Alright, thank you. Then that's probably what I should have included in the documentation instead of the incorrect garbage I wrote :)

[yrodiere]

Uh, wait. The transaction scope is only destroyed after the transaction is committed.

So something like this just wouldn't work, would it?

void beforeDestroyed(@Observes(during = Transaction.BEFORE_COMPLETION) @BeforeDestroyed(TransactionScoped.class) Object event)

[Ladicek]

The funny thing is that in Quarkus, the lifecycle events of the transaction CDI context do not actually follow the lifecycle super-precisely :-)

If you look at the subclasses of TransactionScopedNotifier, we actually fire @BeforeDestroyed(TransactionScoped.class) before we start commit/rollback, even though the context will still be active for the duration of commit/rollback, and we fire @Destroyed(TransactionScoped.class) after commit/rollback finishes.

The transaction CDI context is destroyed in a Synchronization.afterCompletion(), which I guess happens as part of the commit/rollback process.

I guess it would be valid to fire the @BeforeDestroyed(TransactionScoped.class) and @Destroyed(TransactionScoped.class) events also in Synchronization.afterCompletion(), just before/after we destroy the context, but the existing implementation should be good too. In fact, this behavior is probably intentional (catering to the same use cases this issue is about), but I'm not the authority on that.