From 5cecfc7d0bcdc71c8293642999138b5333263900 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Tue, 22 Sep 2026 20:29:19 +0100 Subject: [PATCH] fix(insight): Read the StatsD polling gate under the lock it pairs with onTimer read polling_ before taking metricsLock_. A tick that read the flag as set could be preempted before acquiring the lock, letting onCollectionStopping() clear the flag, take the uncontended lock and return. The tick then resumed and ran every hook handler, which read the ledger master, the network operations, the peer finder, the job queue and the overlay after shutdown had been told polling stopped. Collector::onCollectionStopping() promises polling has stopped by the time it returns, and the shutdown ordering in ApplicationImp depends on that. Reading the gate inside the lock makes seeing it set imply holding the lock, so either the tick holds it across the handlers and the stop waits, or the stop wins and the tick polls nothing. The old comment assumed that invariant rather than establishing it. --- src/libxrpl/beast/insight/StatsDCollector.cpp | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/libxrpl/beast/insight/StatsDCollector.cpp b/src/libxrpl/beast/insight/StatsDCollector.cpp index a1b535035b..9cf0daf2e7 100644 --- a/src/libxrpl/beast/insight/StatsDCollector.cpp +++ b/src/libxrpl/beast/insight/StatsDCollector.cpp @@ -277,8 +277,9 @@ public: { polling_.store(false, std::memory_order_release); - // onTimer holds metricsLock_ across the handler loop, so acquiring it - // here waits for a handler that is already running. + // onTimer reads polling_ under metricsLock_, so a tick that is going to + // call handlers already holds it. Taking it here waits for that tick, + // and any later one reads the cleared flag and polls nothing. std::scoped_lock const _(metricsLock_); } @@ -464,12 +465,17 @@ public: return; } - if (polling_.load(std::memory_order_acquire)) { + // Read the gate under the lock. A tick that sees it set therefore + // holds metricsLock_, which is what lets onCollectionStopping() + // wait for the handlers by taking the same lock. std::scoped_lock const _(metricsLock_); - for (auto& m : metrics_) - m.doProcess(); + if (polling_.load(std::memory_order_acquire)) + { + for (auto& m : metrics_) + m.doProcess(); + } } // The gate above holds back hook handlers, not socket I/O. Events reach