allthingssecurity opened a new pull request, #29789: URL: https://github.com/apache/camel/pull/29789
# Description [CAMEL-26885](https://issues.apache.org/jira/browse/CAMEL-26885) Follow-up of #28804 (CAMEL-26448, the supervising route controller deadlock on route removal), named in its description. `DefaultSupervisingRouteController` keeps the routes it controls in a `TreeSet` (`routes`) and the ids of the routes it does not supervise in a `HashSet` (`nonSupervisedRoutes`). They are changed and read from several threads without one lock: `ManagedRoutePolicy.onInit` adds to both without a lock (a route added at runtime: Kamelets created on demand, route templates, a route reload), `onRemove` removes under the policy's own service lock, and `startRoute`, `stopRoute`, `suspendRoute`, `resumeRoute` and `getControlledRoutes()` read `routes` without a lock (from JMX, the control bus, the developer console, a route reload). `getNonControlledRouteIds()` hands out a live (unmodifiable) view of the `HashSet`. So a route added or removed while another thread looks routes up can fail that thread with a `ConcurrentModificationException` (or `IllegalStateException: Accept exceeded fixed size`, from `toList()` of a `TreeSet` that changed meanwhile), or make a lookup by id miss a route that is there, so it is started without the supervision. Change: `routes` is a `ConcurrentSkipListSet` with the same ordering (startup order, then the order of initialization, `RouteHolder.compareTo` as before) and `nonSupervisedRoutes` a `ConcurrentHashMap` key set, so readers get weakly consistent iteration and no lock is added (taking the controller lock in `onInit`, which runs under the route lock of the CamelContext, is what CAMEL-26448 had to avoid). A `CopyOnWriteArraySet`, as `ClusteredRouteController` uses, keeps the insertion order, not the startup order. `getNonControlledRouteIds()` still returns an unmodifiable view, now of a concurrent set. A lookup by id costs the same as before (a stream over the routes); nothing changes per message. Not changed, for a follow-up JIRA I will offer: an exhausted restart task can still put a route that is being removed into `exhausted` (CAMEL-26448 narrowed this: the callback checks under the controller lock that the task is still the restart task of the route, but `onRemove` releases the route under the policy lock, between that check and the put), and the callback reads the status of a route that may be gone meanwhile. Tests, new `DefaultSupervisingRouteControllerConcurrentRoutesTest`: - `testNonControlledRouteIdsWhileARouteIsAdded`: iterating `getNonControlledRouteIds()` while a route that is not supervised is added (deterministic, one thread); - `testControlledRoutesWhileRoutesAreAddedAndRemoved`: one thread reads the controlled and the non-controlled routes while the test thread adds and removes 100 pairs of routes (a stress test, and the only one for `routes`; the first test shows the `nonSupervisedRoutes` case deterministically). Without the change both fail on main (`ConcurrentModificationException`, and `IllegalStateException: Accept exceeded fixed size of 3` in the reader). With it the camel-core tests pass (9183 run, 0 failures, 0 errors, 45 skipped), and the new class passed 20 runs in a row. No documentation or upgrade guide change. # 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): [CAMEL-26885](https://issues.apache.org/jira/browse/CAMEL-26885). # 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 the core modules with the formatter and import-sort plugins and ran the camel-core tests. 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]
