allthingssecurity opened a new pull request, #29119:
URL: https://github.com/apache/camel/pull/29119
## CAMEL-26535: camel-kamelet - fix deadlock between addRoutes and a toD
creating a Kamelet route
Found with a TLA+ model of the route-management locks of camel-core and
camel-kamelet, then reproduced on the real classes.
### The problem
Two threads take the same two locks in opposite orders:
- **Adding routes while the CamelContext runs** (`addRoutes`, a route
reload, `addRouteFromTemplate` or templated routes, rest definitions added at
runtime): `DefaultModel.addRouteDefinitions` holds the lock of the model while
it creates the new route. The route's `kamelet:a` endpoint is initialized
there, and its `doInit` creates the Kamelet's route in
`KameletComponent.LifecycleHandler.createRouteForEndpoint`, which takes the
**lock of the Kamelet component**. Order: model, then Kamelet component.
- **An exchange sending to a new Kamelet endpoint**
(`toD("kamelet:b?x=${header.x}")`, a `ProducerTemplate`, a recipient list)
initializes the endpoint with no lock held, so `createRouteForEndpoint` takes
the **lock of the Kamelet component** first, and `addRouteFromKamelet` ->
`addRouteDefinitions` then takes the **lock of the model**. Order: Kamelet
component, then model.
Both are `ReentrantLock.lock()`, so both threads hang for good, and so does
everything that needs the lock of the model afterwards (adding and removing
routes, every later Kamelet route). Shortest trace of the model:
```
1 toD thread : createRouteForEndpoint takes the Kamelet component lock
(KameletComponent:687)
2 add thread : addRouteDefinitions takes the model lock
(DefaultModel:196)
3 add thread : reifying the new route -> kamelet:a doInit -> wants the
Kamelet component lock
toD thread : addRouteFromKamelet -> addRouteDefinitions -> wants the model
lock -> deadlock
```
### The fix
The Kamelet component creates a Kamelet's route holding the lock of the
model instead of a lock of its own, as `DefaultCamelContext.removeRoute`
already takes the model lock first to avoid a deadlock with `addRoutes`. On
every path that creates a route, the component lock was always followed by the
model lock, so merging them adds no lock order, and Kamelet route creation
stays serialized. The model lock is now held for the whole creation (including
loading the template from its location), which can make route add/remove wait a
little longer while a Kamelet's route is created.
The model does not expose its lock, so `Model` gets `getModelLock()`
(`DefaultModel` returns its lock, `DefaultCamelContext` delegates). No default
implementation: a lock of its own would exclude nothing. Classes that extend
`DefaultCamelContext` (Spring, Quarkus) inherit it; the upgrade guide has a
note for custom `Model` implementations.
With the change the TLA+ model has no deadlock and every thread returns
(also with a third thread stopping a route under the supervising route
controller), and a SPIN model of the same cycle agrees.
### Tests
`KameletAddRoutesDeadlockTest` (new) forces the interleaving with two hooks
of the public API: a `ModelLifecycleStrategy` (runs while `addRoutes` holds the
model lock) and a route template converter for Kamelet `b` (runs while the toD
creates the Kamelet's route). The hooks only wait and count down. On main both
threads hang and `ThreadMXBean.findDeadlockedThreads()` reports each waiting
for the `ReentrantLock` the other holds; with the change both complete and both
routes work. A second test is a control (the same operations one after the
other). With the change the class ran 20 times in one JVM, 40 tests, 0
failures. Without the change to `KameletComponent` the first test fails with
the deadlock.
Without hooks, adding 200 routes with new Kamelet endpoints while another
thread sends 200 messages through `toD` to new Kamelet endpoints hung in each
of 9 runs on main, and completed in 10 of 10 runs with the change.
camel-kamelet: 154 tests, 3 skipped; the camel-core and camel-main route
template, add/remove route, reload, route controller, model and dump tests
pass. The only failures (`KameletRouteChecksumTest`,
`RouteWatcherReloadChangedRoutesTest`, `BrowseRemoveRouteTest`) fail the same
on main in our local build (older snapshot jars).
### Not part of this
In the same model, `stopRoute` holds the route lock of the CamelContext
while it waits for the route's in-flight exchanges, and an in-flight exchange
of that route that sends to a new Kamelet endpoint needs that lock to start the
Kamelet's route, so the stop is forced after the shutdown timeout. That is a
separate design question; I will file it as a follow-up JIRA.
Affected: main and the maintenance branches (the same two lock orders on
4.23.x, 4.22.x, 4.18.x, 4.14.x). This PR is for main only: it adds a method to
a public interface, and on 4.18.x and 4.14.x the model uses `synchronized`,
which camel-kamelet cannot reach without an API change. A backport would need
its own approach.
_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]
