davsclaus commented on code in PR #27796:
URL: https://github.com/apache/camel/pull/27796#discussion_r4266923830
##########
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:
Non-blocking. Before this change the backing list only grew, so a reader
could not see it shrink. Now it shrinks from `browseLimit + 1` back to
`browseLimit` here. Readers that check the size and then call `get(index)` can
hit `IndexOutOfBoundsException` if this line runs in between. Examples are
`BrowsableEndpoint.getBrowseStatus` (`list.get(list.size() - 1)`) and the index
loops in `ManagedBrowsableEndpoint`.
The window is tiny, and on main the same readers fail with CME as soon as
the limit is reached, so this is still a clear improvement. A possible
follow-up is to let those readers work on a snapshot (iterator or `toArray()`
of the copy-on-write list). That is not needed for this PR.
--
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]
