[
https://issues.apache.org/jira/browse/CAMEL-25062?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18119951#comment-18119951
]
shashank commented on CAMEL-25062:
----------------------------------
PR: https://github.com/apache/camel/pull/26945
The PR changes when the routes of a {{ClusteredRoutePolicy}} are started and
stopped (shortly after the leadership event, on a policy thread that exits when
idle, instead of inside the event), so feedback on the approach is welcome
before it is merged.
I cannot assign issues to myself; could a committer assign this to me
(smjainblr)? Thanks.
_Claude Code on behalf of allthingssecurity_
> camel-cluster - ClusteredRoutePolicy: CamelContext stop or removeRoute can
> deadlock with a leadership change that is starting routes (follow-up to
> CAMEL-24545)
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-25062
> URL: https://issues.apache.org/jira/browse/CAMEL-25062
> Project: Camel
> Issue Type: Bug
> Components: camel-core
> Reporter: shashank
> Priority: Minor
>
> CAMEL-24545 reported a JVM that hangs on shutdown:
> {{ClusteredRoutePolicy.onRemove}} and the ZooKeeper leader selector thread
> take the same two locks in opposite order. The fix (PR #25842, in 4.18.5,
> 4.22.1 and 4.23.0) changed only {{ZooKeeperClusterView}}, so that it no
> longer fires a leadership event while it stops. The reviews on that PR
> described the root cause as the AB-BA inversion between the
> {{ClusteredRoutePolicy}} lock and the {{StampedLock}} of
> {{AbstractCamelClusterView}}, accepted the ZooKeeper guard as the minimal
> fix, and suggested a follow-up for the other cluster views. The inversion
> itself is still in {{ClusteredRoutePolicy}} on main, for every cluster
> service (file, kubernetes, consul, infinispan, jgroups-raft) and for
> ZooKeeper events other than the stop event.
> h3. The lock orders (main, 65f315628)
> * *Event dispatch.* {{AbstractCamelClusterView.fireLeadershipChangedEvent}}
> calls every listener while it holds the read lock of the view
> ({{AbstractCamelClusterView.java:111-122}}). The listener of the policy
> ({{ClusteredRoutePolicy.java:376}}) calls {{setLeader}}, which takes the
> policy lock ({{:267}}) and starts or stops the routes on the same thread.
> {{startRoute}} ({{:310}}) needs the CamelContext route lock
> ({{AbstractCamelContext.startRoute}}).
> * *Release.* {{ClusteredRoutePolicy.releaseClusterView}} is called from
> {{onRemove}} of the last route of the policy and from {{doShutdown}}, so it
> runs on every CamelContext stop and on every route removal or reload. It
> takes the policy lock ({{:235}}) and, while it holds it, calls
> {{removeEventListener}} ({{:239}}), which needs the write lock of the view
> ({{AbstractCamelClusterView.java:104}}).
> * *removeRoute.* {{AbstractCamelContext.removeRoute}} holds the CamelContext
> route lock while it shuts the route down and calls {{onRemove}}.
> {{ClusteredRoutePolicyFactory}} creates one policy per route, and each one
> registers its own listener. A leadership event runs the listeners one after
> the other, and each one starts its route inside the dispatch, with the read
> lock of the view held. Two cycles follow:
> # *CamelContext stop* (two locks). The stop releases a policy further down
> the list: it takes that policy's lock and waits for the write lock of the
> view. The dispatch reaches that policy's listener and waits for the same
> policy lock.
> # *removeRoute* (three locks). The operator holds the CamelContext route lock
> and waits in {{releaseClusterView}} for the policy lock (or, once that is
> lock-free, for the write lock of the view). The dispatch holds the read lock
> of the view and the policy lock, and waits in {{startRoute}} for the
> CamelContext route lock.
> This also applies to a single policy shared by several routes: it holds its
> own lock across {{startRoute}}, while {{removeRoute}}, holding the
> CamelContext route lock, needs the same lock.
> The window is a leadership change that is still starting routes, for example
> a consumer that takes a while to connect to a broker. A rolling restart
> produces exactly this: A stops, B takes over and starts its routes, and B is
> then stopped or has a route reloaded.
> With the file cluster service the stuck thread is the leadership thread of
> {{FileLockClusterService}}. It stops writing the heartbeat but keeps the file
> lock, so no other node can become the leader until the JVM is killed (for
> example by the orchestrator after its stop timeout).
> h3. Reproduction
> Two nodes share one {{FileLockClusterService}} root. Each registers
> {{ClusteredRoutePolicyFactory.forNamespace("ns")}} and has two routes:
> "slow", whose consumer takes 1.5 s to start, and "other". A is the leader. A
> is stopped, so B takes the leadership and its view dispatches the event; the
> policy of "slow" starts its route. 300 ms later B's CamelContext is stopped,
> or "other" is removed.
> CamelContext stop, blocked in 3 of 3 runs:
> {noformat}
> CamelContext.stop() of B STILL BLOCKED after 10006 ms
> thread 'FileLockClusterService-B' WAITING at
> ClusteredRoutePolicy.setLeader:267
> from
> ClusteredRoutePolicy$CamelClusterLeadershipListener.leadershipChanged:376,
> AbstractCamelClusterView.fireLeadershipChangedEvent
> thread 'operator' WAITING at LockHelper.doWithWriteLock:81
> from AbstractCamelClusterView.removeEventListener:104,
> ClusteredRoutePolicy.releaseClusterView:239
> {noformat}
> removeRoute("other"), blocked:
> {noformat}
> removeRoute(other) STILL BLOCKED after 10001 ms
> thread 'FileLockClusterService-B' WAITING at
> AbstractCamelContext.startRoute:1318
> from InternalRouteController.startRoute:124,
> DefaultRouteController.startRoute:133
> thread 'operator' WAITING at ClusteredRoutePolicy.releaseClusterView:235
> from ReferenceCount.release:71, ClusteredRoutePolicy.onRemove:199
> {noformat}
> The removeRoute interleaving is racy in this harness. The unit test in the PR
> makes both cases deterministic: a consumer whose start waits on a latch, and
> a gate listener that holds the dispatch until the view is being released.
> h3. Proposed fix
> Making the release lock-free is not enough: with {{releaseClusterView}}
> taking no policy lock, removeRoute still deadlocks, now between the write
> lock of the view (operator, holding the CamelContext route lock) and the
> CamelContext route lock (dispatch, holding the read lock of the view). The
> routes must not be started or stopped on the thread that dispatches the
> event. The proposed change to {{ClusteredRoutePolicy}}:
> * The leadership listener hands the change to the policy's own thread, which
> starts or stops the routes under the policy lock. The view lock is then never
> held while the CamelContext route lock is awaited, and a slow route start no
> longer blocks the leadership thread of the cluster service (the file
> service's heartbeat).
> * Threading: the thread comes from a pool created through the
> {{ExecutorServiceManager}} (named and managed like the other Camel pools)
> with no core thread, at most one thread and a keep-alive of one second. One
> thread at most keeps the changes of a policy in order, and it exits when the
> leadership does not change, so {{ClusteredRoutePolicyFactory}} (a policy per
> route) does not keep a thread per route, and a removed route leaves no thread
> behind. One shared thread per CamelContext was not used: it would need an
> owner with its own lifecycle, and a slow route start would delay the changes
> of every other policy.
> * The policy thread reads the leadership when it applies a change, so the
> last change wins, and at most one change is queued. A change is never applied
> on the dispatching thread: after the policy has been shut down it is
> rejected, logged and ignored. Errors while applying a change are logged.
> * {{retainClusterView}} and {{releaseClusterView}} take no policy lock. The
> view reference is swapped atomically, and the release only resets the leader
> flag, as no route is left to stop. Retain and release are serialized by a
> separate small lock that only the threads adding and removing routes take, so
> a shared policy cannot release a view that a concurrent route addition has
> just retained. The policy thread ignores a change for a view that has been
> released in the meantime.
> * {{retainClusterView}} reads the current leadership right away, so
> {{onInit}} still lets the route controller start a route added while the node
> is the leader.
> * The deferred start after the CamelContext has started is checked under the
> policy lock, as the policy thread may now apply the leadership while the
> CamelContext is starting.
> * Leadership changes, taken or lost, that reach the policy thread while the
> CamelContext is stopping are ignored, in line with CAMEL-24545. The
> CamelContext is stopping all the routes, consumers first, and stopping a
> route from the policy thread would run a second shutdown of it through the
> same {{ShutdownStrategy}}.
> Behaviour change: the routes are started and stopped shortly after the
> leadership event, on the policy thread, instead of inside it. Most cluster
> services fire the event from their own threads, but some fire it on the
> calling thread (when a view starts or a listener is added). Code that fires
> the event itself (as some tests do) should wait for the route status.
> {{isLeader}} (JMX) reflects the view right away, and the routes follow.
> An alternative is to iterate over a snapshot of the listeners in
> {{AbstractCamelClusterView.doWithListener}}, so no view lock is held during
> the callbacks. That changes every cluster view and camel-master, so it is
> only mentioned as an option.
> A TLA+ model of the view lock and two policy locks shows the two-lock cycle.
> It does not model the CamelContext route lock, so it does not cover the
> removeRoute case.
> h3. Follow-up
> Once this is fixed, views could notify the listeners that are still
> registered when a view is force-stopped (JMX {{stopView}}, or stop of the
> cluster service). Today a leader whose view is force-stopped keeps its
> clustered and master routes running while another node takes over (file and
> ZooKeeper). This is left for a separate issue.
> Affected: long-standing. In 4.4.0 {{releaseClusterView}} and {{setLeader}}
> were {{synchronized}} methods and the view already used a {{StampedLock}}, so
> the lock order was the same; CAMEL-20199 (4.8.0) replaced the monitor with a
> {{ReentrantLock}}. Main, 4.22.x, 4.18.x and 4.14.x are affected. The
> ZooKeeper stop trigger is fixed in 4.18.5 and 4.22.1, but other ZooKeeper
> events and all other cluster services still hit it.
> Duplicate check (2026-09-27): JIRA text "ClusteredRoutePolicy" returns 8
> issues. CAMEL-24545 is the ZooKeeper-only fix of the same inversion,
> CAMEL-24626 fixed the same pattern in camel-master, and CAMEL-17090 (2021) is
> a different lock-up. GitHub PRs for "ClusteredRoutePolicy": #25842 touches
> only {{ZooKeeperClusterView}}. No open PR changes {{ClusteredRoutePolicy}} or
> {{AbstractCamelClusterView}}.
> _Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
