allthingssecurity opened a new pull request, #26860: URL: https://github.com/apache/camel/pull/26860
# Description [CAMEL-25002](https://issues.apache.org/jira/browse/CAMEL-25002) The Loop EIP could break graceful shutdown. A single message with a negative loop count, or an inflight exchange with a huge loop count, made the shutdown strategy stop waiting for inflight exchanges. The route was then stopped at once, and the inflight exchange failed with a `RejectedExecutionException`. `LoopProcessor` added the evaluated count to its pending task counter without checking the sign. A count of 0 or less runs no iteration, and nothing ever compensated a negative add. So after one message with `loop(header("n"))` and `n=-1`, `getPendingExchangesSize()` returned `-1` for the rest of the route's life. `DefaultShutdownStrategy` adds the pending sizes of a route's services to its inflight count, in `int`, and waits only while the sum is positive. The same `LoopProcessor` is a child of several route services and is counted once for each (4 times in a plain route), so the negative value cancelled real inflight exchanges. A count from about 2^29 had the same effect, because the `int` sum overflowed. This regressed in CAMEL-16794, which switched to a `LongAdder` with an unconditional `add(count)` and dropped the `Math.max(count - index, 0)` clamp that CAMEL-15578 had. It is the same symptom as CAMEL-18713 (`loopDoWhile`), whose fix did not cover the negative-count path. This change: - `LoopProcessor` only adds a positive count to the pending counter. The `LOOP_SIZE` property is unchanged. - `LoopProcessor` releases the iterations left exactly once whenever the loop ends. Before, it only did so when the loop ended because of an exception (the release CAMEL-19738 added). A loop that broke out on shutdown (`breakOnShutdown`) left its remaining iterations pending, so the shutdown waited for its full timeout. `LoopBreakOnShutdownTest` now takes 1.1 s instead of 10.1 s, which was its 10 s shutdown timeout. - The remaining iterations are released in one `add(-gap)` call. The old code ran one `decrement()` per iteration, which takes more than 10 s on the routing thread for a count near `Integer.MAX_VALUE`. - `getPendingExchangesSize()` keeps the sum between 0 and `Integer.MAX_VALUE`. `LongAdder.intValue()` wrapped. - `DefaultShutdownStrategy.getPendingInflightExchanges` sums in a `long`, ignores negative sizes so that one service cannot cancel the pending exchanges of others, and caps the result. The per-route sum with the inflight repository size is capped the same way. I did not de-duplicate the services that are counted more than once in `getPendingInflightExchanges`. With the sums capped, that only makes the numbers in the "Waiting as there are still N inflight and pending exchanges" log message higher, as before. Changing it would also change the counts reported for the aggregator and the wire tap. Tests: new `LoopPendingExchangesShutdownTest`: - `testNegativeCountLeavesNoPendingTasks`: counts `-1`, `0` and `3` leave `getPendingExchangesSize() == 0`. - `testGracefulShutdownWaitsForInflightAfterNegativeCount`: one message with `n=-1`, then `context.stop()` while a second exchange is inflight. The stop must wait and the exchange must complete. - `testGracefulShutdownWaitsForInflightWithHugeCount`: the same with a `breakOnShutdown` loop and `n=Integer.MAX_VALUE`, and after the stop the loop has no pending iterations left. The inflight exchanges wait with Awaitility until the context is stopping, and the tests use no sleeps. With the fix they pass deterministically. Without it, the second test relies on the shutdown finishing its first pass before the inflight exchange wakes up, which it practically always does. Without the fix, all 3 fail: ``` testNegativeCountLeavesNoPendingTasks expected: <0> but was: <-1> testGracefulShutdownWaitsForInflightAfterNegativeCount expected: <null> but was: <java.util.concurrent.RejectedExecutionException> testGracefulShutdownWaitsForInflightWithHugeCount expected: <null> but was: <java.util.concurrent.RejectedExecutionException> ``` With only the `DefaultShutdownStrategy` part of the fix (`LoopProcessor` unchanged), the first and third tests still fail (`expected: <0> but was: <2147483646>` for the iterations a `breakOnShutdown` loop left pending). With the fix, all 3 pass. `Loop*,*Shutdown*,AsyncLoop*,*PendingExchanges*` in camel-core: 67 tests, 0 failures, 2 skipped (the existing `*ManualTest`s). Found with a Lean 4 model of the loop's pending counter and the shutdown strategy's wait decision, then reproduced against the real classes. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] 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). # 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 core modules, including the formatter and import-sort plugins. I did not run the full root build. No generated files are affected.) # 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) on behalf of allthingssecurity. The commit carries a `Co-Authored-By` trailer. 🤖 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]
