shashank created CAMEL-26448:
--------------------------------
Summary: camel-core - Supervising route controller: removing a
route that is being restarted can deadlock with its restart attempt (or a
manual start/stop of another route)
Key: CAMEL-26448
URL: https://issues.apache.org/jira/browse/CAMEL-26448
Project: Camel
Issue Type: Bug
Components: camel-core
Reporter: shashank
Assignee: shashank
Found with a TLA+ model of the lock order between
{{DefaultSupervisingRouteController}} (its {{lock}}) and the route lock of the
CamelContext ({{AbstractCamelContext.lock}}), and reproduced on the real
classes.
*What happens*
{{context.removeRoute("a")}} takes the route lock of the CamelContext and shuts
the route down. The route policy of the supervising route controller
({{ManagedRoutePolicy.onRemove}}) then calls {{routeManager.release(route)}}
(added by CAMEL-25071 in 4.23.0), which cancels the restart task of the route
if the route is being restarted (it failed to start and waits for its next
attempt). {{BackOffTimerTask.cancel()}} completes the task in the calling
thread, so the {{whenComplete}} callback of the controller runs in the removing
thread, still under the route lock, and takes the lock of the controller.
The controller holds its lock while it calls into the CamelContext, which takes
the route lock: the restart attempt ({{lock.lock()}}, re-check, then
{{super.startRoute}}), and the manual {{startRoute}} / {{stopRoute}} /
{{suspendRoute}} / {{resumeRoute}} of the routes it controls ({{doStartRoute}}
/ {{doStopRoute}}). So:
# The restart attempt of route a takes the lock of the controller, re-checks
that its task is still active, and waits for the route lock.
# {{removeRoute("a")}} holds the route lock, shuts down the services and the
endpoint of route a, calls {{onRemove}}, cancels the task, and waits for the
lock of the controller in the {{whenComplete}} callback.
Both threads wait forever. {{removeRoute}} never returns, the route lock is
never released, so every later route operation and the CamelContext stop hang
as well. The same happens when another thread is in
{{routeController.stopRoute("b")}} (or start/suspend/resume of a route the
controller knows) instead of the restart attempt. A route that is not being
restarted has no task to cancel and is not affected.
It can happen when a route that keeps failing to start (a broker or database
that is down) is removed: a route reload in dev mode, a Kamelet or route
template removed at runtime, or {{removeRoute}} from user code, while its
restart attempt runs: the restart attempt has to start in the window between
the removal taking the route lock and cancelling the task, which is as long as
the removal takes to shut the route down (close its connections).
*TLA+ model* (each action is one locked step of the code): threads
removeRoute(a), the restart attempt, a user stop of route b; locks S
(controller) and C (route lock). {{NoDeadlock}} is violated in 8 states:
{noformat}
1 rm: lock C removeRoute("a") (ACC:1638)
2 rm: shut down services/endpoint of route a (RouteService:328-338)
3 rm: onRemove -> release(a) (RouteService:341, DSRC:969)
4 rt: isRunAllowed() check (DSRC:681)
5 rt: lock S (DSRC:691)
6 rt: re-check: task still active (DSRC:695)
7 rm: task.cancel() (status Inactive) (DSRC:773,
BackOffTimerTask:138)
8 rm: whenComplete wants S (DSRC:724) | rt: wants C (super.startRoute,
ACC:1435) -> deadlock
{noformat}
The same with the user stop of route b instead of the restart attempt. With the
fix (the callback takes the lock only for an exhausted task that is still the
restart task of the route; release() removes the task before it cancels it) the
model has no deadlock and every operation terminates ({{Termination}}, weak
fairness per thread) with all three threads; witnesses show the restart attempt
still starts the route, and sees the cancellation when it comes first. Note:
{{ManagedRoutePolicy.onRemove}} itself calls {{lock.lock()}}, but that {{lock}}
is the policy's own lock inherited from {{BaseService}} (it hides the field of
the controller), so it is not part of the cycle; a mutant where it is the
controller's lock deadlocks in the model with a user operation on another route
that runs at the same time.
*Reproduced* on main with a test that holds the removal while it shuts down the
endpoint of route a (under the route lock) until the other thread waits for the
route lock: {{ThreadMXBean.findDeadlockedThreads()}} reports the two threads
waiting for each other's {{ReentrantLock}}, for the restart attempt of the
removed route and for a manual stop of another route. Without any hook, a probe
removed a route that fails to start and is restarted every millisecond: with an
endpoint that shuts down at once, 0 deadlocks in 300 removals; with an endpoint
that takes 20 ms to shut down (as closing a connection does), 200 deadlocks in
200 removals on main, and 0 in 200 with the change.
*Proposal*
In the {{whenComplete}} callback take the lock only when the task is exhausted
and is still the restart task of the route (the only case that needs it: it
moves the route out of the supervision), and check both again under the lock.
{{release()}} removes the task from the map before it cancels it, so a
cancelled task completes without the lock, and {{removeRoute}} no longer takes
the lock of the controller under the route lock. A manual stop that races the
last restart attempt still leaves the route not exhausted, as before.
Affected: 4.23.0 and later (the release in {{onRemove}} came with CAMEL-25071);
older lines do not cancel the task under the route lock.
_Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
