gnodet-bot commented on code in PR #28640:
URL: https://github.com/apache/camel/pull/28640#discussion_r4409702069
##########
components/camel-jcache/src/main/java/org/apache/camel/component/jcache/JCacheManager.java:
##########
@@ -37,13 +39,23 @@
import org.apache.camel.util.ObjectHelper;
public class JCacheManager<K, V> implements Closeable {
+ // The caching provider is a JVM singleton, and it returns the same cache
manager for the same uri, which returns
+ // the same cache for the same name: other managers (the other endpoints
of the cache, other caches, repositories,
+ // other CamelContexts) use them too, so each one is closed by the last
manager that uses it
+ private static final Map<Object, Integer> USERS = new HashMap<>();
Review Comment:
💡 The inline comments above explain the ref-counting rationale well, but the
class has no Javadoc documenting the **lifecycle contract**: callers must
`close()` this manager; if they don't, the USERS entry (and the underlying
provider/cache manager/cache) will never be released for the lifetime of the
JVM.
Consider adding a class-level Javadoc (above `public class JCacheManager`)
to make this contract explicit, e.g.:
> Manages JCache resources with JVM-wide reference counting. **Callers must
call `close()`** to release their share of the underlying caching provider,
cache manager, and cache. Failing to do so permanently leaks the USERS entry.
This is especially useful since `JCacheManager` is also used directly by the
idempotent/aggregation repositories, where try-with-resources is less common.
--
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]
