allthingssecurity opened a new pull request, #27796: URL: https://github.com/apache/camel/pull/27796
# Description [CAMEL-25609](https://issues.apache.org/jira/browse/CAMEL-25609) Scope first: `browseLimit` on the browse endpoint is an advanced option, off by default (0 = keep everything), on a component that is used for testing and debugging. Nothing in Camel sets it; the only caller of `setBrowseLimit` on a browse endpoint is `BrowseComponent`, which copies its own option of the same name (also 0 by default). Only users who set the option are affected. The code is unchanged since it was added in 4.9.0 (CAMEL-21183). With `browseLimit`, `BrowseEndpoint.onExchange` made room for a new exchange with `exchanges = exchanges.subList(space, max)` and then added to that sub list. A sub list is a view of the `CopyOnWriteArrayList` behind it, and adding to the view appends to that list; nothing was ever removed from it. Two effects, both reproduced with tests on main: 1. **Exchanges are not released.** `browse:foo?browseLimit=10` showed the last 10 exchanges to a browser but kept every exchange ever sent to it in memory (1000 after 1000 messages), although the option is described as the "Maximum number of messages to keep in memory available for browsing". The endpoint grows until it is stopped, and every add copies the whole, growing array. 2. **A stale view fails, with two producers for good.** A sub list of a `CopyOnWriteArrayList` throws `ConcurrentModificationException` once the backing list was changed through another view. - A list returned by `getExchanges()` could not be read any more after one further exchange arrived. - Two threads sending to the same endpoint: thread A creates its view, thread B creates its own and adds, then A stores its view, which is now outdated. A's add fails, the outdated view stays in the field, and from then on every exchange sent to the endpoint and every read of it (`getExchanges().size()`, the management bean, the browse status) fails with `ConcurrentModificationException` until the endpoint is restarted. The test holds A exactly at that point, so this is reproduced by construction and not by load. This failure is loud: the exception goes to the route's error handling. The fix adds the exchange and then removes the oldest ones from the list itself (`exchanges.subList(0, excess).clear()`, one array copy), both under a lock so that concurrent producers keep exactly `browseLimit` exchanges. The lock is only taken when `browseLimit` is set; without it (the default) nothing changes. The filter is not touched. What changes for users of the option: the memory is released and the exceptions are gone. What a browser sees is the same as before, the last `browseLimit` exchanges in arrival order (`BrowseLimitTest` is unchanged and passes). Two details: - While a new exchange arrived, a reader on main could see one exchange fewer than the limit, and none at all with `browseLimit=1` (the view was stored before the add; `BrowsableEndpoint.getBrowseStatus` then reports size 0). The fix adds first, so a reader never sees fewer than the limit once it was reached; for that moment it can see `browseLimit + 1`. - `getExchanges()` returns the endpoint's own list, as it already did without a limit and before the limit was reached. A caller that keeps the list now sees later exchanges instead of the exception. There is no upgrade guide entry: no option, default or result changes, a route that worked keeps working with the same browsed content, and the only behaviour that goes away is the retained memory and the exception. Cost per exchange with a limit: one lock (uncontended with one producer) and, once the limit is reached, two array copies of at most `browseLimit + 1` entries (add, remove) where main made one copy of all exchanges ever sent. Tests: - `BrowseLimitMemoryTest` in camel-core, 5 tests. All 5 fail without the change (main's `BrowseEndpoint`, same test class) and pass with it: - `testOldExchangesAreReleased`: `browseLimit=10`, 1000 messages, the list that holds the exchanges is handed to the test by `createExchangeList`. Main: `expected: <10> but was: <1000>`. - `testBrowsedListStaysUsableWhenMoreExchangesArrive`: `browseLimit=2`, `getExchanges()`, one more message, then the list from before is read. Main: `ConcurrentModificationException`. - `testSecondProducerWhileFirstMakesRoom`: two producers with the interleaving above, forced by a delegating list that holds the first producer when it asks for the sub list until the second has completed (main) or waits for the lock (fix); then a third exchange and a read. Main: producer A, the later exchange and the read all fail with `ConcurrentModificationException`. - `testManyProducerThreads`: 4 threads, 250 exchanges each, `browseLimit=10`; no failure, 10 browsable and 10 kept. This one depends on scheduling; on main it failed in each of the 3 attempts of the test run (all 4 threads with the exception), but it is not a failure by construction. - `testReaderDoesNotSeeTheEndpointEmptyWithLimitOne`: `browseLimit=1`, the browse status is read while the producer is inside the list. Main: sizes seen `[1, 0, 1, 0]`. - Full camel-core suite with the change: 8171 run, 0 failures, 0 errors, 45 skipped. - camel-management, the browse tests (`*Brows*Test`): 19 run, 0 failures. How it was found: a boundary sweep with a small Lean 4 model of the window (main: backing list and offset of the view; fix: one list). It proves that main keeps every exchange for any limit, that the fixed list never exceeds `browseLimit`, and that the fix shows a browser exactly what main showed. The exception, concurrent producers and the moment between add and remove are not part of the model; they are covered by the tests above. Not changed: `getExchanges()` creating the list lazily without synchronization, and sending to a stopped endpoint; both are as on main. The other `BrowsableEndpoint` implementations (seda, mock, file, jms, activemq) have their own `browseLimit` code and no sub list that replaces the stored list; they are not touched. Not run: the full camel-management suite and the full root build. The release branches were read, not run: `camel-4.23.x`, `camel-4.22.x`, `camel-4.18.x` and `camel-4.14.x` have the same line. # 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 camel-browse, camel-core and camel-management with the modules they depend on, including the formatter and import-sort plugins, and ran the full camel-core test suite and the tests named above. 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]
