davsclaus commented on code in PR #27796:
URL: https://github.com/apache/camel/pull/27796#discussion_r4270404610


##########
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:
   Please do the reader fix in this PR rather than as a follow-up. The trim is 
new in this PR, so it should come with the readers it can break.
   
   Let `BrowsableEndpoint.getBrowseStatus` and the index loops in 
`ManagedBrowsableEndpoint` read one snapshot of the copy-on-write list 
(`toArray()` or an iterator) instead of calling `size()` and then `get(index)`. 
Please add a test for that too.
   
   _Claude Code on behalf of davsclaus_



-- 
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]

Reply via email to