allthingssecurity commented on PR #28640: URL: https://github.com/apache/camel/pull/28640#issuecomment-6494117587
@oscerd thanks for the close read. I checked the concurrency point against the code. `getCache()` already takes the manager's per-instance `lock` before the `closed` check, and holds it through the `cache == null` init and the three `retain(...)` calls (`JCacheManager.java:93-112` at 69df4da; on main the lock is at `:74-84`). `close()` takes the same lock (`:117-138`). So concurrent first calls from `JCacheProducer.process()` run the init and the retains once per manager. `retained` and `cache` are only touched under that lock. 3f93009 makes this explicit and pins it: - A comment in `getCache()` explains why the lock is needed. - `JCacheManagerTest.concurrentFirstCallsOfGetCacheCountTheManagerOnce`: 16 threads call `getCache()` together on a fresh manager, over 20 rounds. All of them must get the same cache, and `close()` must close it. With the lock taken out of `getCache()`, it failed in 20 of 20 runs (about 1 round in 3; the count stays above 0 and the cache stays open). With the lock it passed 20 of 20. - `JCacheRemoveRouteTest.routesAndEndpointsThatAreStartedAgainKeepWorking` covers your second point. A route stopped and started again keeps its endpoint's manager. An endpoint stopped and started again gets a new manager, while the old one throws `IllegalStateException`, and sends work again. camel-jcache: 100 tests, 0 failures. No production logic change. _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]
