fix(data): fire transaction synchronizations, and scope them to their transaction #9

2026-08-12T23:25:36Z

Relism commented

2026-08-12 23:25:36 +00:00

Four defects in the same mechanism, all silent, plus the contract that was missing.

1. `afterCommit` never fired on the Hibernate path

HibernateTxManager#commit fired synchronizations after its finally block, where cleanupIfIdle() had already called ResourceRegistry.cleanup() and removed the ThreadLocal list holding them. fireSynchronizations() then read a freshly initialized empty list and did nothing — no exception, no log, just silence. rollback() twenty lines below had the order right, which is what makes this an ordering slip rather than a design choice.

Found from the far end, in production: an admin write landed in Postgres while the in-memory cache it was registered to refresh never heard about it, so the change only took effect once the process restarted and re-read the database at boot.

2. Synchronizations were not scoped to their transaction

They lived in a single flat per-thread list, fired from index 0 by whichever transaction completed first. A REQUIRES_NEW inner transaction therefore fired the suspended outer transaction's callbacks too — early, with the inner transaction's outcome, for a transaction that might still roll back. Each new transaction now records how many synchronizations existed when it began and fires only its own tail.

3. `JdbcTxStatus` rejected a null connection

Which turned the two propagations that deliberately produce a connectionless status — SUPPORTS with no active transaction, and NOT_SUPPORTED — into an NPE inside begin(). The Hibernate manager has always allowed it, and resource() already reports the real mistake with a message that names it.

4. `beforeCommit` was declared and invoked by nothing

Part of the TxSynchronization API from the start, called from neither manager: anyone implementing it got silence, the same failure class as 1. It now runs immediately before the real commit, with the transaction still active and its session/connection still bound — the whole reason to have a hook on that side of the commit is that it can still write through the same resource and land in the same atomic unit. Skipped when the transaction is already rollback-only.

Throwing from it vetoes the commit: the transaction rolls back, the surviving callbacks hear ROLLED_BACK, and the exception propagates. Without that, a hook running before the commit would be strictly less useful than one running after. A commit that fails on its own now takes the same path instead of completing silently with no callback at all.

Ordering

Both managers now share it: unbind the session/connection first, so a post-completion callback that opens its own transaction (a cache reload, an outbox drain) gets a fresh one instead of joining the transaction that just committed — then fire, then clean up.

Documentation

The full lifecycle contract is now written down on TxSynchronization itself and in flash-ext-data-core/docs/README.md: which hook sits on which side of the commit, what each may still touch, what throwing does, and the per-transaction scoping rule.

Tests

22 new, kept deliberately parallel across the two managers since they are interchangeable behind TxManager: synchronization firing, callback ordering, per-transaction scoping, callbacks opening their own transaction, beforeCommit running inside the transaction / receiving the read-only flag / being skipped on a rollback-only transaction / vetoing the commit, and the previously untested SUPPORTS/NOT_SUPPORTED/MANDATORY propagations. Nothing covered afterCommit before, which is how these shipped. 37 tests across the two managers, all green; the full reactor verify passes the 80% Jacoco gate.

Four defects in the same mechanism, all silent, plus the contract that was missing. ### 1. `afterCommit` never fired on the Hibernate path `HibernateTxManager#commit` fired synchronizations *after* its `finally` block, where `cleanupIfIdle()` had already called `ResourceRegistry.cleanup()` and removed the ThreadLocal list holding them. `fireSynchronizations()` then read a freshly initialized empty list and did nothing — no exception, no log, just silence. `rollback()` twenty lines below had the order right, which is what makes this an ordering slip rather than a design choice. Found from the far end, in production: an admin write landed in Postgres while the in-memory cache it was registered to refresh never heard about it, so the change only took effect once the process restarted and re-read the database at boot. ### 2. Synchronizations were not scoped to their transaction They lived in a single flat per-thread list, fired from index 0 by whichever transaction completed first. A `REQUIRES_NEW` inner transaction therefore fired the *suspended* outer transaction's callbacks too — early, with the inner transaction's outcome, for a transaction that might still roll back. Each new transaction now records how many synchronizations existed when it began and fires only its own tail. ### 3. `JdbcTxStatus` rejected a null connection Which turned the two propagations that deliberately produce a connectionless status — `SUPPORTS` with no active transaction, and `NOT_SUPPORTED` — into an NPE inside `begin()`. The Hibernate manager has always allowed it, and `resource()` already reports the real mistake with a message that names it. ### 4. `beforeCommit` was declared and invoked by nothing Part of the `TxSynchronization` API from the start, called from neither manager: anyone implementing it got silence, the same failure class as 1. It now runs immediately before the real commit, with the transaction still active and its session/connection still bound — the whole reason to have a hook on that side of the commit is that it can still write through the same resource and land in the same atomic unit. Skipped when the transaction is already rollback-only. Throwing from it **vetoes the commit**: the transaction rolls back, the surviving callbacks hear `ROLLED_BACK`, and the exception propagates. Without that, a hook running before the commit would be strictly less useful than one running after. A commit that fails on its own now takes the same path instead of completing silently with no callback at all. ### Ordering Both managers now share it: unbind the session/connection first, so a post-completion callback that opens its own transaction (a cache reload, an outbox drain) gets a fresh one instead of joining the transaction that just committed — then fire, then clean up. ### Documentation The full lifecycle contract is now written down on `TxSynchronization` itself and in `flash-ext-data-core/docs/README.md`: which hook sits on which side of the commit, what each may still touch, what throwing does, and the per-transaction scoping rule. ### Tests 22 new, kept deliberately parallel across the two managers since they are interchangeable behind `TxManager`: synchronization firing, callback ordering, per-transaction scoping, callbacks opening their own transaction, `beforeCommit` running inside the transaction / receiving the read-only flag / being skipped on a rollback-only transaction / vetoing the commit, and the previously untested `SUPPORTS`/`NOT_SUPPORTED`/`MANDATORY` propagations. Nothing covered `afterCommit` before, which is how these shipped. 37 tests across the two managers, all green; the full reactor `verify` passes the 80% Jacoco gate.

Relism added 1 commit 2026-08-12 23:25:36 +00:00

fix(data): fire transaction synchronizations, and scope them to their transaction 7299490d0d

Two defects in the same mechanism, both silent.

1. HibernateTxManager#commit fired synchronizations *after* its finally block,
   where cleanupIfIdle() had already called ResourceRegistry.cleanup() and
   removed the ThreadLocal list holding them. fireSynchronizations() then read
   a freshly initialized empty list and did nothing. No afterCommit callback
   had ever run on the Hibernate path: no exception, no log, just silence.
   rollback() twenty lines below had the order right, which is what makes this
   an ordering slip rather than a design choice.

   Found in production, from the far end: an admin write landed in Postgres
   while the in-memory cache it was registered to refresh never heard about it,
   so the change only took effect when the process restarted and re-read the
   database at boot.

2. Synchronizations were a single flat per-thread list, fired from index 0 by
   whichever transaction completed first. A REQUIRES_NEW inner transaction
   therefore fired the *suspended* outer transaction's callbacks too — early,
   with the inner transaction's outcome, for a transaction that might still
   roll back. Each new transaction now records how many synchronizations were
   already registered when it began, and fires only its own tail.

Both managers get the fix and the same callback ordering: unbind the session or
connection first, so a callback that opens its own transaction (a cache reload,
an outbox drain) gets a fresh one instead of joining the transaction that just
committed, then fire, then clean up.

Also fixes JdbcTxStatus rejecting a null connection, which turned the two
propagations that deliberately produce a connectionless status — SUPPORTS with
no active transaction, and NOT_SUPPORTED — into an NPE inside begin(). The
Hibernate manager always allowed it, and resource() already reports the real
mistake with a message that names it.

Tests: 16 new across the two managers, kept deliberately parallel since the two
are interchangeable behind TxManager — synchronization firing, ordering,
per-transaction scoping, callbacks opening their own transaction, and the
previously untested SUPPORTS/NOT_SUPPORTED/MANDATORY propagations. Nothing
covered afterCommit before, which is how both defects shipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Relism added 1 commit 2026-08-12 23:33:12 +00:00

feat(data): actually invoke TxSynchronization.beforeCommit, and document the contract 0194470c1f

beforeCommit(boolean) has been part of the TxSynchronization API from the
start and was invoked by nothing, in either manager — anyone implementing it
got silence, the same failure class as the dropped afterCommit callbacks in
the previous commit. Left out of that one because fixing it is a design
decision rather than a restored behaviour; this is that decision, written
down.

It now runs immediately before the real commit, with the transaction still
active and its session/connection still bound, which is the whole reason to
have a hook on this side of the commit: it can still write through the same
resource and land in the same atomic unit. Skipped when the transaction is
already rollback-only, since there is no commit to precede.

Throwing from it vetoes the commit: the transaction rolls back, the surviving
callbacks hear ROLLED_BACK, and the exception propagates. Without that, a hook
running before the commit would be strictly less useful than one running
after. A commit that fails on its own now takes the same path instead of
completing silently with no callback at all, and a rollback that also fails is
attached as a suppressed exception rather than replacing the one that explains
the failure.

Documented on the interface itself and in flash-ext-data-core/docs/README.md:
which hook sits on which side of the commit, what each may still touch, what
throwing does, and the per-transaction scoping rule from the previous commit.

Tests: 6 more (3 per manager) — runs inside the transaction with the resource
still bound, receives the read-only flag, skipped on a rollback-only
transaction, and vetoes the commit when it throws. 37 across the two managers
now, all green, reactor verify passes the 80% Jacoco gate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Relism added 1 commit 2026-08-12 23:36:19 +00:00

docs(data): write the data-layer docs in English c5179146c0

The three data modules' docs were in Italian, so the synchronization contract
added in the previous commit went in as Italian too, to match its file. English
is the project's language for docs, comments and READMEs alike, and a file half
in each is worse than either — so all three are translated, not just the new
section.

Content is otherwise unchanged, except the "synchronizations run on
commit/rollback" line in the two backend READMEs, which was vague before and is
now accurate about which hook sees the session/connection still bound, pointing
at flash-ext-data-core's README for the full contract.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Relism merged commit 8f1f30b973 into master

2026-08-12 23:37:36 +00:00

Relism deleted branch fix/tx-synchronizations

2026-08-12 23:37:36 +00:00

Relism referenced this issue from a commit

2026-08-12 23:42:36 +00:00

chore(deps): bump Flash to 2.1.0-8f1f30b, and surface unsupported-backend 502s

Relism referenced this issue from a commit

2026-08-12 23:43:17 +00:00

chore(deps): bump Flash to 2.1.0-8f1f30b, and surface unsupported-backend 502s

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Relism/Flash5#9