allthingssecurity opened a new pull request, #27523: URL: https://github.com/apache/camel/pull/27523
# Description [CAMEL-25066](https://issues.apache.org/jira/browse/CAMEL-25066) Fixes the follow-up @davsclaus filed after the review of #26862 (CAMEL-25004): endpoints are shared per CamelContext (toD, recipientList, routingSlip and ProducerTemplate resolve the same `Endpoint` instance from the endpoint registry), but each of them has its own `DefaultProducerCache`. When one cache evicts the producer of a singleton endpoint, `ServicePool` (`SinglePool.cleanupEvicts`) stops the endpoint and removes it from the registry unless `isEndpointInUse()` finds it static or consumed by a route. It does not know about the other producer caches, so the endpoint is stopped while another cache still holds a started producer of it. What users see depends on the endpoint. `SedaEndpoint.stop()` is a no-op while it has producers, so with seda the endpoint is only removed from the registry (the other cache keeps a producer of an orphaned endpoint and creates a second one on its next resolve). Endpoints that release resources in `doStop` break: `HttpEndpoint.doStop` closes its `HttpClient`, so an in-flight or later send of the other route (or of a ProducerTemplate that keeps the `Endpoint`) fails. Fix (reference counting through what CamelContext already tracks): every pooled singleton producer (and polling consumer) is added as a service to CamelContext (`addService(producer, true, true)` in `SinglePool.acquire`, listed when the producer is a singleton, which `DefaultProducer` takes from its endpoint), and removed when it is stopped. `isEndpointInUse()` now also returns true while another such service of the endpoint is registered (`CamelContext.hasService(s -> s instanceof EndpointAware ea && ea.getEndpoint() == endpoint)`), and `cleanupEvicts` stops the evicted pool first, so its own producer no longer counts. A dynamic endpoint that nothing else uses is still stopped and removed to free its resources, as before. I did not leave the stopping to the endpoint registry: `AbstractDynamicRegistry` deliberately does not stop the endpoints it evicts ("may still be in use"), so dynamic endpoints would no longer be stopped at all. Found and checked with a TLA+ model (`tla/r17t/producer-cache/ProducerCacheEviction.tla`, kept outside the repo) of two or three producer caches sharing one dynamic uri: resolve from the registry (a new endpoint instance after removal), acquire (pool, producer creation as a service), process, release, and a send to another uri that evicts the producer and runs `cleanupEvicts` (check, stop endpoint, remove from registry, stop pool), with one sender thread per cache. On main TLC violates "no exchange is sent with a stopped endpoint" (B holds a producer, A evicts and stops the endpoint, B sends: 20 steps) and "no cache keeps a live producer of a stopped endpoint", also with B keeping the `Endpoint` reference (ProducerTemplate). With the fix all properties hold, including "a started endpoint is registered or used by a producer" (dynamic endpoints are still freed), for 2 and 3 caches that all evict, up to 8 operations. Negative controls (one cache; a route consuming from the endpoint) hold on main. Cost: the extra check scans the CamelContext services, only when a producer cache evicts the producer of a dynamic endpoint (not per message), and only after the static and route checks. Measured roughly with a `toD` whose cache evicts on every send (cacheSize 10, 100 seda uris): about 23 us per send on main and with the change at ~30 services; with 1,000 extra `EndpointAware` services 25 us (main) vs 33 us; with 10,000, 24 us vs 99 us. A plain loop instead of `hasService(Predicate)` made no difference. Only contexts with thousands of services whose producer caches evict constantly would notice. What the check does not see (as on main, these do not keep the endpoint started): producers that are not pooled as context services, such as those of a `toD` with `cacheSize=-1` (`EmptyProducerCache`, created per send), and producers created directly with `endpoint.createProducer()`. And when the endpoint is kept because another cache still has a producer, it is only stopped later if that other cache evicts it too; if that cache is stopped instead (route removed, template stopped), the endpoint stays started like any endpoint of a stopped producer cache today. Not changed, and not caused by this ticket (the model shows both on main and with the fix): a cache that creates its first producer of an endpoint instance it resolved just before, while another thread's `cleanupEvicts` is between the in-use check and stopping the endpoint, gets a producer of a stopped endpoint; this also happens with two threads of one route. And a ProducerTemplate that keeps an `Endpoint` and is the only user still gets a stopped endpoint after its own cache evicted it. Closing these needs a lock shared by the caches of an endpoint, which I left out to keep this change small. No upgrade guide entry: an endpoint is now only stopped on eviction when nothing else uses it. Tests: new `ProducerCacheEvictSharedEndpointTest`: - `testEvictDoesNotStopEndpointOfOtherRoute`: two routes with `toD`, route a with `cacheSize` 1 evicts `seda:shared` while route b still caches its producer; - `testEvictDoesNotStopEndpointOfProducerTemplate`: a test endpoint whose producer fails when its endpoint is stopped (like an HTTP endpoint with a closed client), used through a ProducerTemplate with the `Endpoint` instance; - `testEvictStopsEndpointNotInUseWhenPoolIsNotStoppedOnEviction`: a dynamic endpoint used by one cache only is still stopped and removed when the cache has no more pools than its capacity (two producers of a non-singleton endpoint), so `onEvict` does not stop the evicted pool and only `cleanupEvicts` does. It passes on main, and is kept because it is the only test of the new order in `cleanupEvicts`: with `p.stop()` moved back after the in-use check it fails (`the dynamic endpoint not in use should be stopped to free resources ==> expected: <false> but was: <true>`), as the evicted pool's own producer then counts as a user. In the other tests `onEvict` already stops the pool. That a dynamic endpoint used by one cache only is still stopped and removed is also covered by the existing `ProducerCacheEvictEndpointInUseTest.testEvictStopsDynamicEndpointNotInUse`. The first two fail on main in two runs (`seda:shared should still be registered ==> expected: <seda://shared> but was: <null>`, `The template should still be able to send to closing:shared ==> expected: <null> but was: <java.lang.IllegalStateException: Endpoint is stopped: closing://shared>`). `camel-support`: 125 tests, 0 failures; `camel-core`: 8073 tests, 0 failures, 1 error (45 skipped). The error is `ValidatorExternalResourceTest`, which loads an XSD from raw.githubusercontent.com and could not connect from this machine (`SocketTimeoutException: Connect timed out`), unrelated to this change. From an earlier run with the same production code: `camel-management` producer/consumer cache tests (`ManagedConsumerCacheTest`, `ManagedProducer*Test`, `ManagedRecipientListTest`, `ManagedSendDynamicProcessorTest`): 8 classes, 0 failures; `CaffeineCacheProducerCacheNameTest`, 0 failures. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it). # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built and tested `core/camel-support` and `core/camel-core`, including the formatter and import-sort plugins. No generated files change. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
