From 70ec64e7046e33c2085a1bc2283d1b3be091aa4f Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Tue, 22 Sep 2026 19:05:41 +0100 Subject: [PATCH] fix(telemetry): Leave no queued handler in the StatsD loopback test server receive() built its result in a local and captured it by reference. A handler only ever runs inside the io_context, so cancel() does not retire the pending receive, it queues an operation_aborted completion. That completion survived into the next receive() call still holding a reference to the previous call's destroyed local. Nothing read it, because the handler skips on a non-zero error code, but the helper could not safely be called twice and its own example shows two calls. The result is now a member cleared per call, and the aborted completion is drained before returning, so no handler is queued when receive() returns. That makes the counter test's positive control possible. It asserted only that nothing arrived, which passed just as happily when the channel was dead: pointing the collector at a wrong port did not fail it. An untouched gauge publishes its initial zero, so the test now asserts that exact datagram arrives before asserting the counter stays silent. --- .../libxrpl/beast/insight/StatsDCollector.cpp | 34 ++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/src/tests/libxrpl/beast/insight/StatsDCollector.cpp b/src/tests/libxrpl/beast/insight/StatsDCollector.cpp index 5364b2b442..f9a9ac7346 100644 --- a/src/tests/libxrpl/beast/insight/StatsDCollector.cpp +++ b/src/tests/libxrpl/beast/insight/StatsDCollector.cpp @@ -72,6 +72,9 @@ public: /** * Waits for one datagram. * + * Leaves no handler queued, so a later call cannot run this call's + * completion. + * * @param timeout How long to wait before giving up. * @return the datagram's bytes, or an empty string if none arrived in * time. @@ -79,17 +82,22 @@ public: std::string receive(std::chrono::milliseconds timeout) { - std::string received; + received_.clear(); socket_.async_receive( boost::asio::buffer(buffer_), - [&received, this](boost::system::error_code const& ec, std::size_t bytes) { + [this](boost::system::error_code const& ec, std::size_t bytes) { if (!ec) - received.assign(buffer_.data(), bytes); + received_.assign(buffer_.data(), bytes); }); ioContext_.restart(); ioContext_.run_for(timeout); + + // A handler runs only inside the io_context, so cancel() merely queues + // one carrying operation_aborted. Run the context again to retire it. socket_.cancel(); - return received; + ioContext_.restart(); + ioContext_.poll(); + return received_; } private: @@ -108,6 +116,12 @@ private: * 1472-byte packet limit. */ std::array buffer_{}; + + /** + * What the last receive() read, empty when its wait ran out. A member, so + * no queued handler can outlive what it writes to. + */ + std::string received_; }; /** @@ -135,6 +149,10 @@ TEST(StatsDCollector, UntouchedGaugePublishesInitialZero) * This is the other half of the rule above, and it is why the fix is a gauge * starting dirty rather than a flush of everything on the first tick. A counter * reports events, so an unsent counter and a zero counter mean the same thing. + * + * Silence is also what a dead loopback channel looks like, so an untouched gauge + * comes along as a positive control. One flush tick serves both metrics, so a + * counter line would have to travel beside the gauge's. */ TEST(StatsDCollector, UntouchedCounterPublishesNothing) { @@ -143,8 +161,14 @@ TEST(StatsDCollector, UntouchedCounterPublishesNothing) auto collector = StatsDCollector::make(address, "test", Journal(Journal::getNullSink())); auto const counter = collector->makeCounter("untouched"); + auto const control = collector->makeGauge("control"); - // Three seconds spans several one-second flush ticks. + // The control's line, alone: the channel carries a datagram, and the + // counter contributed nothing to it. + EXPECT_EQ(server.receive(std::chrono::seconds(10)), std::string("test.control:0|g\n")); + + // Three seconds spans several one-second flush ticks. The control is clean + // after its first flush, so anything arriving now is the counter. EXPECT_EQ(server.receive(std::chrono::seconds(3)), std::string()); }