davsclaus commented on code in PR #25842:
URL: https://github.com/apache/camel/pull/25842#discussion_r3874711092
##########
components/camel-zookeeper/src/main/java/org/apache/camel/component/zookeeper/cluster/ZooKeeperClusterView.java:
##########
@@ -151,7 +151,9 @@ public void takeLeadership(CuratorFramework
curatorFramework) throws Exception {
task.run(getCamelContext(), () -> !isRunAllowed());
} finally {
leader = false;
- fireLeadershipChangedEvent((CamelClusterMember) null);
+ if (isStoppingOrStopped()) {
+ fireLeadershipChangedEvent((CamelClusterMember) null);
+ }
Review Comment:
This condition looks inverted. The JIRA ticket (CAMEL-24545) proposes
guarding this event with `if (!isStoppingOrStopped())` — i.e. **skip** firing
the event when the view is stopping/stopped, because by the time
`doStop()`/`selector.close()` runs, `ClusteredRoutePolicy.releaseClusterView()`
has already removed its listener, so the event is unneeded, and firing it here
is exactly what causes the AB-BA deadlock (this thread ends up blocked in
`selector.close()` while the Curator leader thread blocks trying to acquire
`ClusteredRoutePolicy.lock` inside the listener callback).
As written (`if (isStoppingOrStopped())`), the event fires *only* during
shutdown — the deadlock-prone path is still exercised, so the deadlock isn't
actually fixed — and is now suppressed during normal (non-shutdown) leadership
loss, which is a new regression: listeners like `ClusteredRoutePolicy` will no
longer be notified when leadership is lost outside of shutdown.
```suggestion
if (!isStoppingOrStopped()) {
fireLeadershipChangedEvent((CamelClusterMember) null);
}
```
--
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]
