allthingssecurity opened a new pull request, #27921: URL: https://github.com/apache/camel/pull/27921
# Description [CAMEL-25682](https://issues.apache.org/jira/browse/CAMEL-25682) CAMEL-24134 added a write guard so that the task of the circuit breaker does not write its result into the exchange after a timeout handed the exchange to the fallback. The task claims the guard with `compareAndSet(false, true)` and then copies its result (`ResilienceProcessor.processTask`, and `processTaskAsync` in asynchronous mode). The fallback task only did `exchangeWriteGuard.set(true)` before writing the exchange. When the timeout fires after the task has won the guard and before it has finished copying, the two write the same exchange at the same time: - with `onFallback`, the body of the fallback is replaced by the body of the task, or `CamelCircuitBreakerResponseFromFallback` is set back to `false` (and `CamelCircuitBreakerResponseSuccessfulExecution` to `true`) after the fallback set them, while the exchange already continues in the route; - without fallback, `ExchangeHelper.copyResults` ends with `setException(null)` and clears the `TimeoutException` that the caller has just set; - headers and properties are written by two threads. The caller does not wait for the task here (the task runs in `CompletableFuture.supplyAsync` behind `TimeLimiter.decorateFutureSupplier`, or `decorateCompletionStage` in async mode), so the window is the time the task needs to copy its result; it needs a task that ends at about the moment of the timeout. This change replaces the `AtomicBoolean` by a small per-call guard with a lock. The task claims it and copies its result while holding the lock; the fallback task claims it under the same lock. A caller that times out while the task is copying waits until the copy is done (`copyResults` and two properties). It then sets the timeout result or runs the fallback on the exchange, which at that point holds the result of the task (its body, headers and properties), as on main when the timeout fires just after the copy. The guard only decides whether the task writes: it writes only if it claims the exchange before the caller; the caller always writes after a timeout. Cost: one `ReentrantLock` per call instead of one `AtomicBoolean`, and an uncontended lock/unlock on the write-back. In asynchronous mode the "caller" of a timed-out call is a thread of the time limiter's scheduler pool (by default the same pool that runs the tasks); the fallback already runs there, and that thread can now w ait for a write-back in progress, which is bounded by the copy. Not changed: - camel-microprofile-fault-tolerance has the same `set(true)`, but it is not affected: with SmallRye Fault Tolerance the caller gets the `TimeoutException` only after the task has returned. The same test, adapted to that module, passes on main (the caller waited for the held task). - When `copyResults` itself throws after the task claimed the exchange, the exception is dropped and the half-copied exchange counts as a success (the second claim in the `catch` fails). main has the same flaw with the `AtomicBoolean`; follow-up. - Whether a caller that finds the write-back already claimed should keep the result of the task instead of running the fallback: that would change the outcome of such a call (a success after the breaker recorded a timeout), so it is left as a question, not changed here. - With the pooled exchange factory, a task that is still running after the timeout uses a task object that the caller has already given back to its pool; that is a separate issue and not changed here. No upgrade guide entry: no option, default or output changes; the only difference is that a caller can wait for a write-back that is in progress. Tests: - New `ResilienceTimeoutDuringWriteBackTest` (4 tests: sync and `asynchronous=true`, with and without `onFallback`, timeout 500 ms). The result message of the task holds the first read of its body by `ExchangeHelper.copyResults` (the test checks the stack, so the hold is inside the write-back) until the caller has returned, or has not returned 2 seconds after the timeout. The tests also check that the result is the one of the fallback or the timeout, set on the exchange that holds the result of the task (a header the task set). Without the change all 4 fail: `expected: <Fallback result> but was: <Task result>` and `expected: <java.util.concurrent.TimeoutException> but was: <null>`, sync and async. With the change each test takes about 2.5 s (the caller waits for the held write-back). - Without the hold, a load run that is not part of this PR (20000 calls from 32 threads, timeout 20 ms, a task of 20 ms +/- 0.3 ms) gave 26 inconsistent exchanges on main and 0 with the change (one run each). - camel-resilience4j unit tests with the change: 81 run, 0 failures. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [ ] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it). (The ticket is drafted and not filed yet; CAMEL-25682 is a placeholder until it is.) # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built and tested the affected module, including the formatter and import-sort plugins. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) 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-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
