allthingssecurity commented on PR #28640: URL: https://github.com/apache/camel/pull/28640#issuecomment-6490459541
@davsclaus thanks for the review. 7cfa1947f77f (one new commit on top) covers the two smaller points; for the design I have a proposal rather than a rework. - Upgrade guide: new 4.24 entry. A `JCacheManager` now closes its cache, cache manager and caching provider only when no other `JCacheManager` uses them. `getCache()` after `close()` now throws `IllegalStateException`; before, it returned the closed cache, whose operations throw it. - `JCacheRemoveRouteTest:67` is now `knownLimitationRemovingTheConsumerRouteOfTheSameUriStopsTheEndpointOfItsProducer`. Its javadoc says it pins today's behaviour, not the wanted one, and must expect the sends to work once the endpoint is kept for its producer. Design: I looked at the CAMEL-26302 approach (producers and consumers hold the reference). It fixes only the case where the users share one endpoint, which is the case the known-limitation test pins. The bug this PR fixes is the other one: `JCacheManager.close()` closes the `CachingProvider` that `Caching.getCachingProvider()` shares across the JVM, so one endpoint's stop closes every cache manager of that provider. That hits the caches of other endpoints (other cache names), the jcache repositories, `JCachePolicy` (it takes `Caching.getCachingProvider().getCacheManager()` itself) and other CamelContexts. None of those is a producer or consumer of the stopping endpoint, so per-endpoint references cannot see them; some count across endpoints is needed. camel-ehcache has one too: `EhcacheComponent.managers`, per component. So I did not switch. Options, your pick: 1. Keep the JVM-wide count of this PR. The entries are released by `close()`; a manager that is never closed keeps its three entries, as it keeps its cache open. 2. A count per `JCacheComponent` (per CamelContext), like ehcache's `managers` map: no static state, but two CamelContexts in one JVM can again close each other's provider. 3. Never close the provider and the cache manager (Camel does not create the provider, `Caching` shares it), and count only the caches. This is the smallest blast radius and also protects `JCachePolicy` and application code. But a provider that owns threads (Hazelcast) would stay open after `context.stop()`, a user-visible change for the guide. Whichever you pick, adding producer/consumer references on top (the ehcache approach) would also fix the same-uri case and flip the known-limitation test. I would do that as a follow-up commit or JIRA once the scope is decided. camel-jcache: 98 tests, 0 failures; `JCacheRemoveRouteTest` passed 20 runs in one JVM. _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]
