CAMEL-24133: Circuit Breaker EIP - Release correlated copy instead of original exchange in pooled mode#24796
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>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
There was a problem hiding this comment.
Review: CAMEL-24133 — Circuit Breaker EIP - Release correlated copy instead of original exchange in pooled mode
Verdict: Approve ✅
Clear-cut bug fix. Both circuit breaker implementations create a correlated copy for processing but release the original exchange back to the pool. In pooled mode, release() calls PooledExchange.done() which wipes body, properties, exception, and exchangeId — destroying the still-in-use original exchange.
Code changes are correct:
ResilienceProcessor.processTask()—copyis initialized tonullat line 569, assigned at line 581 viacreateCorrelatedCopy(). The null guard at the release point handles the case where copy creation itself throws (caught by thecatch(Exception e)at line 607).FaultToleranceProcessor.CircuitBreakerTask.call()— identical structure:copyinitialized null (line 410), assigned at line 423, null-guarded release.
Both match the established pattern: create copy → process → ExchangeHelper.copyResults(exchange, copy) → UnitOfWorkHelper.doneUow(uow, copy) → processorExchangeFactory.release(copy).
Tests are well-structured:
Both tests (ResiliencePooledProcessorExchangeFactoryTest and FaultTolerancePooledProcessorExchangeFactoryTest) configure the critical pooled mode that triggers the bug:
context.getCamelContextExtension().setExchangeFactory(new PooledExchangeFactory());
context.getCamelContextExtension().setProcessorExchangeFactory(new PooledProcessorExchangeFactory());The assertion body().isEqualTo("Bye World") directly verifies the body survives the circuit breaker — without the fix, it would be null. Uses MockEndpoint.assertIsSatisfied(context, 30, TimeUnit.SECONDS) per project conventions.
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.
|
🧪 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).
|
Summary
Fixes CAMEL-24133
Both Circuit Breaker EIP implementations (
ResilienceProcessorandFaultToleranceProcessor) release the original in-flight exchange back to the processor exchange pool instead of the correlated copy whencamel.main.exchange-factory=pooledis enabled.ResilienceProcessor.processTask()calledprocessorExchangeFactory.release(exchange)— the originalFaultToleranceProcessor.CircuitBreakerTask.call()— same bugThis causes
PooledExchange.done()to wipe the body, properties, exception, and exchangeId of the still-in-use original exchange, resulting innullbody downstream and potential cross-exchange corruption under concurrency.Fix: Release the correlated
copy(with null guard) instead of the originalexchange, matching the pattern used by Enricher, Splitter, and MulticastProcessor.Changes
ResilienceProcessor.java— changeprocessorExchangeFactory.release(exchange)toprocessorExchangeFactory.release(copy)FaultToleranceProcessor.java— same changeResiliencePooledProcessorExchangeFactoryTest— usestimer:consumer with bothPooledExchangeFactoryandPooledProcessorExchangeFactory, verifies body survives circuit breakerFaultTolerancePooledProcessorExchangeFactoryTest— same test for microprofile-fault-toleranceTest plan
ResiliencePooledProcessorExchangeFactoryTestpasses (fails without fix)FaultTolerancePooledProcessorExchangeFactoryTestpasses (fails without fix)Claude Code on behalf of davsclaus