davsclaus commented on code in PR #26028:
URL: https://github.com/apache/camel/pull/26028#discussion_r3911134943
##########
components/camel-master/src/main/java/org/apache/camel/component/master/MasterConsumer.java:
##########
@@ -103,6 +109,11 @@ protected void doStart() throws Exception {
protected void doStop() throws Exception {
super.doStop();
+ // a start can still be pending, cancel it first so it cannot start
the delegated consumer
+ // after this consumer has been stopped
+ leadershipTaken = false;
+ cancelLeaderTask(true);
+
if (view != null) {
view.removeEventListener(leadershipListener);
Review Comment:
Confirmed via `AbstractCamelClusterView`/`BaseService`: this call needs the
view's `StampedLock` write lock while this thread already holds
`MasterConsumer`'s `BaseService.lock` (from `stop()`). Meanwhile
`AbstractCamelClusterView.doWithListener` dispatches `leadershipChanged` while
holding the view's read lock, and (with this PR) that dispatch unconditionally
acquires `MasterConsumer.lock` even when there's no consumer yet. That's a
lock-order inversion between the two locks.
This ordering pre-dates this PR (the "taken" path already re-entered
`MasterConsumer.lock` from inside dispatch), but this change measurably widens
exposure since `onLeadershipLost` is now dispatched — and takes the lock — on
every lost event rather than only when `delegatedConsumer != null`. The PR
description already discloses this ("Lock ordering between the cluster view and
the consumer"), which is appreciated — suggesting a follow-up JIRA to track it,
since that context won't survive in the source once this merges.
--
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]
