allthingssecurity opened a new pull request, #27341: URL: https://github.com/apache/camel/pull/27341
# Description [CAMEL-25300](https://issues.apache.org/jira/browse/CAMEL-25300) `TotalRequestsThrottler` removes the state of a correlation key 10 periods after its last permit was returned, with `states.remove(key)`. This is the pattern fixed in `ConcurrentRequestsThrottler` by CAMEL-24926 (#26773): - An exchange that looked up the state just before the clean still takes a permit from the removed state, while the next exchanges create a new state with all the permits. More exchanges than allowed pass in that period. - That exchange returns its permit to the removed state, which schedules the removed state's clean again, and 10 periods later that clean removes the state that replaced it (`remove(key)` removes whatever is mapped), while it is in use. Its delayed permits are dropped and the next exchange gets a fresh set again. - The clean period was computed once from the initial time period, so after `setTimePeriodMillis` (JMX) increased the period more than 10 times, the clean removed states whose permits were still delayed. This change, as in CAMEL-24926: - `clean()` only removes its own state (`computeIfPresent(key, (k, s) -> s == this && markRemovedIfUnused() ? null : s)`), and only when all its permits are back in the queue; it marks the state removed under a lock of its own. Unlike CAMEL-24926 this is not the state's lock: here a decrease of the maximum requests (dynamic expression) holds that lock while it waits in `DelayQueue.take()` for a permit to discard, so an exchange that took a permit must be able to check the flag and return the permit meanwhile (the concurrent throttler decreases with `Semaphore.reducePermits`, which does not wait). - An exchange that took a permit from a removed state puts it back unchanged and takes one from the current state (for `poll()` and for the blocking `take()`). - The clean is scheduled with the current time period (10 times, saturated so a huge period does not wrap). This is part of the same fix: "unused" counts the permits in the queue, delayed or not, so the clean must not run while a permit is still delayed, which it could after the period was increased. The defect was found with a TLA+ model of the states, the clean tasks, the exchanges (lookup, poll, enqueue as separate steps) and the time periods: "at most `maximumRequests` permits per period" and "the clean of a replaced state never removes the current state" are violated on the current code and hold with this change. No upgrade guide entry. Tests: new `TotalRequestsThrottlerCleanTest`, built like `ConcurrentRequestsThrottlerCleanTest` (an executor that keeps the scheduled cleans; the `maximumRequests` expression runs them after the throttler looked up its state for a given exchange). Throttle 2 per hour with `rejectExecution`: - the clean runs after an exchange looked up the state: after it, only 2 exchanges pass in the period; - after `setTimePeriodMillis(2 hours)` the clean is scheduled in 20 hours; - the clean still removes an unused state (control). Without the main-code change: ``` TotalRequestsThrottlerCleanTest.testCleanAfterExchangeLookedUpState:64 Expected org.apache.camel.CamelExecutionException to be thrown, but nothing was thrown. TotalRequestsThrottlerCleanTest.testCleanFollowsTimePeriod:94 expected: <[72000000]> but was: <[36000000]> ``` New `TotalRequestsThrottlerRateDecreaseTest`, for the lock above: `throttle(header("max")).timePeriodMillis(2000)`, two exchanges take both permits, two more wait for them (the test waits until their threads are parked), then an exchange with `max=1` decreases the maximum. All five must get through (about 4 s). It passes on main, and hung with the first version of this change (flag under the state's lock; thread dump: two exchanges parked in `isRemoved()`, the decrease in `DelayQueue.take()`). With the change the throttle tests (`org/apache/camel/processor/throttle/**`, `Throttl*Test`) pass: 60 tests, 0 failures. # 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 modules, 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]
