allthingssecurity opened a new pull request, #27524: URL: https://github.com/apache/camel/pull/27524
# Description [CAMEL-25093](https://issues.apache.org/jira/browse/CAMEL-25093) Reported by Randheer Chauhan: with one CamelContext (camel-spring-boot), deployment units are loaded at runtime with `PluginHelper.getRoutesLoader(camelContext).updateRoutes(resource)`, each an XML DSL file with routes and beans. Sequentially it works; from a thread pool it fails with `ConcurrentModificationException` in three places: iterating the XML loader's delayed beans (`XmlRoutesBuilderLoader` `configureCamel`), `DefaultModel.addCustomBean` (`ArrayList.removeIf`) and `SimpleRegistry.bind` (`HashMap.computeIfAbsent`). Cause: none of the involved state is meant for concurrent updates. There is one `XmlRoutesBuilderLoader` per CamelContext. It keeps the beans that failed while pre-parsing in a shared list (`delayedRegistrations`) and registers them again when a resource's routes are configured, so a concurrent update can add to that list while another update iterates it. The delayed bean of one update can also be registered by the other update. `DefaultModel.addCustomBean` and `SimpleRegistry` (a `LinkedHashMap` by API) update plain collections. Camel's own callers already serialize route loading: `RouteWatcherReloadStrategy` reloads from one thread, Kamelet route creation runs under a lock ("creating dynamic routes from kamelets should not happen concurrently so we use locking"), and route definitions are added under the model lock. Fix: `DefaultRoutesLoader.updateRoutes` runs one call at a time, with a lock of its own (the reporter's sequential case, now also when called from several threads). The javadoc of both `RoutesLoader.updateRoutes` methods says so, and that these calls are not serialized with `loadRoutes` or other ways of adding routes. The lock is a `ReentrantLock` (not `synchronized`, as elsewhere in camel-base-engine, so virtual threads are not pinned while routes are loaded); a nested call from the same thread would re-enter it. The only caller in Camel is `RouteWatcherReloadStrategy` (dev mode / camel-main / camel-jbang reload), which holds no lock when it calls it. Making each collection thread-safe would not be enough on its own: the delayed beans are shared across updates by design (a bean can depend on a bean of another resource in the same batch), `SimpleRegistry` is a `LinkedHashMap` subclass, and the model has more lists without locks (route configurations, transformers, validators). Rou te start was already serialized by the model lock, so the change costs only the parsing that could overlap. `loadRoutes` is deliberately not locked. Kamelet template loading calls it while holding the Kamelet lock, and an update can take the Kamelet lock while creating a route with a kamelet endpoint, so locking `loadRoutes` would add a lock-order cycle. Checked with a TLA+ model (`tla/r17t/update-routes/ConcurrentUpdateRoutes.tla`, kept outside the repo) of two or three concurrent updates. It models the fail-fast checks of the three collections (iterator `next()`, `removeIf`, `computeIfAbsent` as begin/end steps with modification counters), the model lock, the Kamelet lock and a thread routing to a new kamelet at runtime. On main TLC reproduces the delayed-bean failure (7 steps), the `computeIfAbsent` failure (7 steps) and a delayed bean registered by the other update. With the lock, every property holds: no `ConcurrentModificationException`, all routes and beans added, each bean registered by its own update, and the loader lock is never taken while holding another lock. Locking `loadRoutes` too breaks that lock order (kamelet thread). One update at a time (the negative control) passes on main. Not covered: concurrent `updateRoutes` calls now wait for each other, including while an update stops a route it replaces. If an exchange of that route is itself blocked calling `updateRoutes` (a route that deploys routes and is updated by another such call), the stop waits for that exchange until the shutdown timeout and then forces the route to stop (before, both calls ran at the same time). A `loadRoutes` running at the same time (such as a Kamelet template loaded at runtime) can still meet the same unsynchronized collections; that is outside this ticket. Upgrade guide: a 3-line note `=== camel-core - RoutesLoader.updateRoutes runs one call at a time`, next to the route reload notes. It says that concurrent calls now wait for each other, and describes the self-update case above (the stop waits until the shutdown timeout). Tests: new `XmlConcurrentUpdateRoutesTest` (camel-xml-io-dsl). It runs two `updateRoutes` with one XML resource each, whose bean fails while pre-parsing; a latch holds the first update while it registers its delayed bean. Without the change it fails in two runs (all reruns) with `The first updateRoutes should not fail ==> Unexpected exception thrown: java.util.concurrent.ExecutionException: java.util.ConcurrentModificationException`, the same stack as the report (`XmlRoutesBuilderLoader$1.configureCamel` -> `RouteBuilder.checkInitialized` -> `updateRoutesToCamelContext` -> `DefaultRoutesLoader.updateRoutes`). With the change the second update waits for the first and both add their routes and beans. Related tests and module suites: camel-core (where camel-base-engine is tested, incl. `RoutesConfigurationUpdateTest` and the `RouteWatcherReloadStrategy*Test`s) 8070 tests, 0 failures (45 skipped); camel-xml-io-dsl 70 tests (all 18 classes), 0 failures; `KameletDiscoveryTest`, camel-se mantic `SemanticDeclarationDslTest` + `SemanticXmlAutoDiscoveryTest` (67) and yaml `SemanticQuestionTest` (31), 0 failures. After the javadoc was added (no code change), rerun built with `-am`: camel-xml-io-dsl 70 tests (18 classes), camel-core `RouteWatcherReloadStrategy*Test` + `RoutesConfigurationUpdateTest` 8, `KameletDiscoveryTest` 2, camel-semantic 67, camel-yaml-dsl `RouteReload*Test` (4 classes, which reload through `updateRoutes`) + `SemanticQuestionTest` 38, 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-api`, `core/camel-base-engine`, `core/camel-core` and `dsl/camel-xml-io-dsl` with their upstream modules, 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]
