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.
This commit is contained in:
Pratik Mankawde
2026-09-22 20:29:19 +01:00
parent e8bb4f4657
commit 5cecfc7d0b

View File

@@ -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