allthingssecurity commented on code in PR #27796:
URL: https://github.com/apache/camel/pull/27796#discussion_r4267169249
##########
components/camel-browse/src/main/java/org/apache/camel/component/browse/BrowseEndpoint.java:
##########
@@ -140,14 +143,22 @@ protected List<Exchange> createExchangeList() {
*/
protected void onExchange(Exchange exchange) throws Exception {
if (browseLimit > 0) {
- // make room if end of capacity
- int max = exchanges.size();
- if (max >= browseLimit) {
- int space = Math.abs(max - (browseLimit - 1));
- exchanges = exchanges.subList(space, max);
+ // add first and then remove the oldest from the list itself: a
sub list is only a view that keeps them
+ // all in memory, and a reader never sees fewer than the limit
once it was reached. Under a lock, as
+ // concurrent producers would otherwise remove too many or
overtake the limit
+ limitLock.lock();
+ try {
+ exchanges.add(exchange);
+ int excess = exchanges.size() - browseLimit;
+ if (excess > 0) {
+ exchanges.subList(0, excess).clear();
Review Comment:
Agreed. The list shrinks from `browseLimit + 1` to `browseLimit` under
`limitLock`, so a reader that takes the size
and then calls `get(size - 1)` (as `BrowsableEndpoint.getBrowseStatus` and
the index loops of
`ManagedBrowsableEndpoint` do) can get an `IndexOutOfBoundsException` in
that window. Any trim has that window
(removing first would shrink to `browseLimit - 1` instead), so the fix
belongs in the readers: they can read one
snapshot (`toArray()` or an iterator of the copy-on-write list). I will do
that as a follow-up with its own test and
leave this PR as it is.
_Claude Code on behalf of allthingssecurity_
--
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]
