shashank created CAMEL-25062:
--------------------------------
Summary: 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
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)
