CAMEL-25005: camel-core - Saga EIP: do not lose the compensation of a step that joins while the in-memory saga ends - #26865
Open
allthingssecurity wants to merge 1 commit into
Conversation
… step that joins while the in-memory saga ends InMemorySagaCoordinator.beginStep checked that the saga was RUNNING, evaluated the step's options, and then added the step to the enlistments, without any lock. complete(), compensate() and the timeout task changed the status with a compareAndSet and then finalized a snapshot of the enlistments. A step that passed the check before the saga ended, but enlisted after the snapshot, got a successful beginStep, so its action ran, but it was never passed to its compensation or completion endpoint. The window includes the option expressions, and the saga timeout makes it reachable with synchronous routes only. beginStep now checks the status again and enlists the step under a lock of the coordinator, and complete(), compensate() and the timeout task change the status and take the snapshot they finalize under the same lock. A step that loses the race fails with "Cannot begin: status is ...", as it does when it arrives a moment later. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
oscerd
reviewed
Sep 25, 2026
oscerd
left a comment
Contributor
There was a problem hiding this comment.
Traced the locking — this correctly closes a real money-safety race in the Saga EIP.
- The single coordinator
locknow serializes the two operations that were previously unordered:beginStep's status-check-plus-enlist, and the status change + enlistment snapshot inend(status). Becauseenddoes thecompareAndSet(RUNNING, …)andnew ArrayList<>(enlistments)under the same lockbeginStepholds while it enlists, every step that successfully began is guaranteed to be in the finalized snapshot — so the "beginStep succeeded (action ran) but the step was never compensated/completed" window is gone. beginStepevaluating the step options outside the lock and then re-checking status under it is the right shape: no user expression runs while holding the coordinator lock, and a step that loses the race is failed withIllegalStateException("Cannot begin: status is …")so its action never runs — identical to arriving a moment later.- Finalization (
doCompensate/doComplete/doFinalize) receives the snapshot and runs outside the lock, so the compensation/completion endpoints don't execute under it. The timeout is scheduled under the lock but its task only takes the lock when it later fires, so there's no nested-lock deadlock, and preventing scheduling aftercancelTimeouts()is a good extra guard. Keeping the publicdoCompensate/doComplete/doFinalizesignatures (delegating with the live list) preserves compatibility while the internal paths use the snapshot.
I don't see a remaining path where a began step escapes finalization. This is also independent of the other two Saga PRs (#26866, #26869) — different files, no reconciliation needed. LGTM.
(CI has not been triggered yet — fork PR awaiting a maintainer to approve the workflow run; I'll confirm green before it merges.)
This review was generated with AI assistance and reviewed/issued by the human operator. Claude Code on behalf of oscerd
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
CAMEL-25005
InMemorySagaCoordinator.beginStepwas check-then-act without a lock:RUNNING;complete(),compensate()and the timeout task switch the status withcompareAndSet(RUNNING, ...), anddoFinalizethen takes a snapshot of the enlistments. Nothing ordered that snapshot after the enlistment of a step that had already passed the check. Such a step:beginStep, so its action ran (for example, the payment was taken);The window covers the option evaluation. The saga timeout makes it reachable with synchronous routes only: the saga times out while a joining step is between the check and the enlistment. Without any forcing (timeout 1 ms, owner work about 1 ms, plain
simpleoption), 1 of 5000 sagas ended compensated with the payment step executed and never compensated.This change:
beginStepstill evaluates the options outside of any lock. Then, under a lock of the coordinator, it checks the status again and enlists the step (and schedules its timeout).complete(),compensate()and the timeout task change the status and take the snapshot they finalize under the same lock. The snapshot is passed to the finalization instead of reading the list later.A step that loses the race now fails with
IllegalStateException("Cannot begin: status is ..."), as it does when it arrives a moment later. The publicdoCompensate/doComplete/doFinalizemethods keep their signatures and behaviour. Timeouts can also no longer be scheduled aftercancelTimeouts()ran.Tests: new
SagaJoinDuringTimeoutTest. AMANDATORYstep joins a saga with a 100 ms timeout. Its option expression waits (Awaitility) until the saga's compensation is running, which puts the enlistment after the snapshot deterministically. Without the fix:(the payment ran, and
mock:compensate-paymentnever received anything). With the fix, the step fails to begin and the test passes. All*Saga*tests in camel-core pass (36 tests). The reproduction:payment step action executed=0, and the stress run found no lost compensations in 5000 sagas.Found with a TLA+ model of the in-memory saga (coordinator, timeout, synchronous and asynchronous participants), then reproduced against the real classes. With this change, "every step whose
beginStepsucceeded is completed or compensated exactly once" holds, along with "never both" and termination.This is independent of CAMEL-25006, the other in-memory saga fix I am sending, where a REQUIRED/SUPPORTS step of a saga that has already ended starts a new saga (a regression from CAMEL-24144). Each fixes its own property in the model, and both together satisfy all of them.
Target
mainbranch)Tracking
Apache Camel coding standards and style
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.(I built and tested the affected modules, including the formatter and import-sort plugins. I did not run the full root build.)
AI-assisted contributions
Co-authored-bytrailers) and the PR description identifies the AI tool used.This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a
Co-Authored-Bytrailer.Claude Code on behalf of allthingssecurity
🤖 Generated with Claude Code