[
https://issues.apache.org/jira/browse/CAMEL-26448?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Claus Ibsen updated CAMEL-26448:
--------------------------------
Fix Version/s: 4.24.0
> 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
> Priority: Major
> Fix For: 4.24.0
>
>
> 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)
