CAMEL-24134: Circuit Breaker EIP - prevent timed-out worker from writing back to exchange#24797
Merged
Conversation
… original exchange in pooled mode Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
…ing back to exchange Both ResilienceProcessor and FaultToleranceProcessor had a race condition where the worker thread could write results back to the original exchange after a timeout triggered fallback processing on the caller thread. This could overwrite fallback results, reinstate cleared exceptions, or corrupt the exchange through non-atomic interleaving of ExchangeHelper.copyResults. Fix: add a shared AtomicBoolean guard between the worker task and the fallback. The first to claim it (via CAS) gets to write to the exchange; the other skips the write-back entirely. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
approved these changes
Jul 16, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Review: CAMEL-24134 — Circuit Breaker EIP - prevent timed-out worker from writing back to exchange
Verdict: Approve ✅
Well-designed concurrency fix. The race condition is real: when a circuit breaker timeout fires, the fallback runs on the caller thread while the worker may still be executing on its thread. Without a guard, the worker can overwrite the fallback result via ExchangeHelper.copyResults() or exchange.setException().
Guard design is correct:
The shared AtomicBoolean exchangeWriteGuard provides mutual exclusion on write-back:
- Worker uses
compareAndSet(false, true)— atomic claim-or-skip before both the success path (copyResults) and the exception path (setException) - Fallback uses
set(true)— unconditionally claims the guard before processing
This handles all orderings correctly:
- Fallback first →
set(true)→ worker's CAS fails → worker skips write-back ✓ - Worker first → CAS succeeds → worker writes, then fallback
set(true)and overwrites → final state is the fallback result, which is the correct outcome ✓ - No timeout → worker CAS succeeds → writes normally, no fallback invoked ✓
Both implementations are consistent:
ResilienceProcessor: guard shared betweenCircuitBreakerTaskandCircuitBreakerFallbackTask, passed toprocessTask()as a parameter. Thecall()andget()methods both forward it correctly.FaultToleranceProcessor: guard set on task field, fallback handled inprocess()catch block withexchangeWriteGuard.set(true)before fallback processing.- Both include null guards for defensive safety, and both include the CAMEL-24133 fix (
release(copy)instead ofrelease(exchange)).
Tests are well-designed:
Both *TimeoutWriteBackRaceTest classes create a deterministic race (3s sleep vs 500ms timeout), verify the fallback result wins, then use a CountDownLatch to wait for the worker to fully complete and re-verify no late write-back occurred.
Minor observation (non-blocking):
The test's process() lambda has misleading indentation — exchange.getIn().setBody("Worker result") appears to be outside the try block visually but is syntactically inside it. The formatter should fix this.
Reviewed with Claude Code (claude-code/1.0.3) on behalf of gnodet. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 34 tested, 29 compile-only — current: 34 all testedMaveniverse Scalpel detected 63 affected modules (current approach: 34).
|
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
2 participants
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.
Summary
Claude Code on behalf of davsclaus
Both
ResilienceProcessorandFaultToleranceProcessorhave a race condition where the worker thread writes results back to the original exchange after a timeout has triggered fallback processing on the caller thread. This can:ExchangeHelper.copyResultsFix: Add a shared
AtomicBooleanguard between the worker task and the fallback task. The worker usescompareAndSet(false, true)before writing back — if the fallback already claimed the exchange, the write-back is skipped. The fallback sets the guard totruebefore touching the exchange.Both the normal result path and the exception catch path in the worker are guarded.
Note: This PR depends on CAMEL-24133 (also included in this branch) which fixes a related pooled exchange factory release issue in the same processors.
Test plan
ResilienceTimeoutWriteBackRaceTest— slow processor (3s) with short timeout (500ms), verifies fallback result survives after worker completesFaultToleranceTimeoutWriteBackRaceTest— same pattern for MicroProfile Fault Tolerance🤖 Generated with Claude Code