From f9b2d725a00328bab6a3f2f625ccd02260554a72 Mon Sep 17 00:00:00 2001
From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
Date: Tue, 21 Jul 2026 11:26:29 +0100
Subject: [PATCH] fix(telemetry): assert SpanGuard Scope is destroyed on its
 constructing thread

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 include/xrpl/telemetry/SpanGuard.h  |  7 ++++++-
 src/libxrpl/telemetry/SpanGuard.cpp | 25 +++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/include/xrpl/telemetry/SpanGuard.h b/include/xrpl/telemetry/SpanGuard.h
index ea8a121a0e..88814bfcb5 100644
--- a/include/xrpl/telemetry/SpanGuard.h
+++ b/include/xrpl/telemetry/SpanGuard.h
@@ -149,6 +149,10 @@
  * it may be moved to and destroyed on another thread; detached()
  * itself must be called on the origin (constructing) thread. Use
  * captureContext() to propagate the trace context to other threads.
+ * Violating this rule is enforced (not just documented): a scoped
+ * guard destroyed on a foreign thread, or detached() called from one,
+ * trips an XRPL_ASSERT in debug/test/fuzzing builds instead of
+ * silently corrupting the other thread's context stack.
  *
  * @note Move semantics: Move construction transfers ownership of
  * the pimpl pointer — no double-Scope issues. Move assignment is
@@ -333,7 +337,8 @@ public:
      *
      * @return A scope-less guard safe to move to and destroy on
      * another thread, or a null guard if this guard was null.
-     * @note Must be called on the origin (constructing) thread. The
+     * @note Must be called on the origin (constructing) thread; this is
+     * checked by an XRPL_ASSERT in debug/test/fuzzing builds. The
      * returned guard may be freely moved across threads; only
      * its final destruction ends the span.
      */
diff --git a/src/libxrpl/telemetry/SpanGuard.cpp b/src/libxrpl/telemetry/SpanGuard.cpp
index b0437b8e17..3b920e51b3 100644
--- a/src/libxrpl/telemetry/SpanGuard.cpp
+++ b/src/libxrpl/telemetry/SpanGuard.cpp
@@ -25,6 +25,7 @@
 
 #include <xrpl/telemetry/SpanGuard.h>
 
+#include <xrpl/beast/utility/instrumentation.h>
 #include <xrpl/telemetry/DiscardFlag.h>
 #include <xrpl/telemetry/Telemetry.h>
 
@@ -44,6 +45,7 @@
 #include <optional>
 #include <string>
 #include <string_view>
+#include <thread>
 #include <typeinfo>
 #include <utility>
 
@@ -89,6 +91,16 @@ struct SpanGuard::Impl
      */
     std::optional<otel_trace::Scope> scope;
 
+    /**
+     * Thread that constructed this Impl. Meaningful only when `scope`
+     * holds a value: a Scope is bound to the thread-local context stack
+     * of the thread that pushed it, so popping it (via ~Scope, when
+     * ~Impl runs) on a different thread corrupts that thread's stack.
+     * Checked in ~Impl() and detached() to turn a silent cross-thread
+     * corruption into an assertion failure in debug/test/fuzzing builds.
+     */
+    std::thread::id owner{std::this_thread::get_id()};
+
     /**
      * Construct a scoped guard: the span is pushed onto this thread's
      * active-context stack for the lifetime of the guard.
@@ -118,6 +130,14 @@ struct SpanGuard::Impl
 
     ~Impl()
     {
+        // A live Scope (scope engaged) must be popped on the thread that
+        // pushed it; ending on any other thread corrupts that thread's
+        // context stack. A detached guard (scope == nullopt) carries no
+        // binding and may be destroyed anywhere, so the check is skipped.
+        XRPL_ASSERT(
+            !scope.has_value() || owner == std::this_thread::get_id(),
+            "xrpl::telemetry::SpanGuard::Impl::~Impl : scoped guard destroyed on "
+            "constructing thread");
         if (span)
             span->End();
     }
@@ -312,6 +332,11 @@ SpanGuard::detached() &&
 {
     if (!impl_)
         return {};
+    // Popping the Scope (below, via impl_.reset()) must happen on the thread
+    // that pushed it; calling detached() elsewhere would pop the wrong stack.
+    XRPL_ASSERT(
+        !impl_->scope.has_value() || impl_->owner == std::this_thread::get_id(),
+        "xrpl::telemetry::SpanGuard::detached : called on constructing thread");
     // Take the span out; the old Impl.span is now null so ~Impl won't End().
     auto s = std::move(impl_->span);
     // Resetting the old Impl destroys its Scope HERE, on the origin thread,