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


##########
core/camel-support/src/main/java/org/apache/camel/support/cache/ServicePool.java:
##########
@@ -130,6 +125,18 @@ public void release(Endpoint endpoint, S s) {
         Pool<S> p = pool.get(endpoint);
         if (p != null) {
             p.release(s);
+        } else {
+            // the pool was evicted while this service was in use
+            stopAndRemove(s);
+        }
+    }
+

Review Comment:
   This is the key fix — previously when the pool was evicted while a service 
was in use, `release()` silently did nothing and the consumer leaked. Now it 
properly stops and removes the orphaned service. :+1:



##########
core/camel-support/src/main/java/org/apache/camel/support/cache/ServicePool.java:
##########
@@ -94,12 +94,7 @@ protected void onEvict(S s) {
             }
         } else {
             // service no longer in a pool (such as being released twice, or 
can happen during shutdown of Camel etc)
-            ServicePool.stop(s);
-            try {
-                e.getCamelContext().removeService(s);
-            } catch (Exception ex) {
-                LOG.debug("Error removing service: {}. This exception is 
ignored.", s, ex);
-            }
+            stopAndRemove(s);

Review Comment:
   Minor nit: `onEvict()` already computed `Endpoint e = getEndpoint.apply(s)` 
at line 87, but `stopAndRemove(s)` recomputes it. The old code reused the 
cached `e`. Since `getEndpoint` is typically a simple field accessor this is 
negligible, but you could pass `e` as a parameter to avoid the redundant lookup.



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