From b04b1d2495d56e893b167ee0cf81e20f1b14e0ec Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Wed, 14 Jan 2026 13:48:32 +0100 Subject: [PATCH 1/7] fix(core): Make HubSwitchGuard !Send to prevent thread corruption HubSwitchGuard manages thread-local hub state but was Send, allowing it to be moved to another thread. When dropped on the wrong thread, it would corrupt that thread's hub state instead of restoring the original thread. To fix this, add PhantomData> to make the guard !Send while keeping it Sync. This prevents the guard from being moved across threads at compile time. Additionally, refactor sentry-tracing to store guards in thread-local storage keyed by span ID instead of in span extensions. This fixes a related bug where multiple threads entering the same span would clobber each other's guards. Fixes #943 Refs RUST-130 Co-Authored-By: Claude --- CHANGELOG.md | 5 +++++ sentry-core/src/hub_impl.rs | 16 ++++++++++++---- sentry-tracing/src/layer.rs | 31 +++++++++++++++++++++---------- 3 files changed, 38 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2621cf3d..50045878e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ - Added a `Envelope::into_items` method, which returns an iterator over owned [`EnvelopeItem`s](https://docs.rs/sentry/0.46.2/sentry/protocol/enum.EnvelopeItem.html) in the [`Envelope`](https://docs.rs/sentry/0.46.2/sentry/struct.Envelope.html) ([#983](https://github.com/getsentry/sentry-rust/pull/983)). +### Fixes + +- Fixed thread corruption bug where `HubSwitchGuard` could be dropped on wrong thread ([#957](https://github.com/getsentry/sentry-rust/pull/957)) + - **Breaking change**: `sentry_core::HubSwitchGuard` is now `!Send`, preventing it from being moved across threads. Code that previously sent the guard to another thread will no longer compile. + ## 0.46.2 ### New Features diff --git a/sentry-core/src/hub_impl.rs b/sentry-core/src/hub_impl.rs index 5786daa36..36994fa18 100644 --- a/sentry-core/src/hub_impl.rs +++ b/sentry-core/src/hub_impl.rs @@ -1,5 +1,6 @@ use std::cell::{Cell, UnsafeCell}; -use std::sync::{Arc, LazyLock, PoisonError, RwLock}; +use std::marker::PhantomData; +use std::sync::{Arc, LazyLock, MutexGuard, PoisonError, RwLock}; use std::thread; use crate::Scope; @@ -19,10 +20,14 @@ thread_local! { ); } -/// A Hub switch guard used to temporarily swap -/// active hub in thread local storage. +/// A guard that temporarily swaps the active hub in thread-local storage. +/// +/// This type is `!Send` because it manages thread-local state and must be +/// dropped on the same thread where it was created. pub struct SwitchGuard { inner: Option<(Arc, bool)>, + /// Makes this type `!Send` while keeping it `Sync`. + _not_send: PhantomData>, } impl SwitchGuard { @@ -41,7 +46,10 @@ impl SwitchGuard { let was_process_hub = is_process_hub.replace(false); Some((hub, was_process_hub)) }); - SwitchGuard { inner } + SwitchGuard { + inner, + _not_send: PhantomData, + } } fn swap(&mut self) -> Option> { diff --git a/sentry-tracing/src/layer.rs b/sentry-tracing/src/layer.rs index d02c0496d..72178001b 100644 --- a/sentry-tracing/src/layer.rs +++ b/sentry-tracing/src/layer.rs @@ -1,6 +1,6 @@ use std::borrow::Cow; use std::cell::RefCell; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; use std::sync::Arc; use bitflags::bitflags; @@ -236,7 +236,6 @@ pub(super) struct SentrySpanData { pub(super) sentry_span: TransactionOrSpan, parent_sentry_span: Option, hub: Arc, - hub_switch_guard: Option, } impl Layer for SentryLayer @@ -338,7 +337,6 @@ where sentry_span, parent_sentry_span, hub, - hub_switch_guard: None, }); } @@ -350,12 +348,15 @@ where None => return, }; - let mut extensions = span.extensions_mut(); - if let Some(data) = extensions.get_mut::() { - data.hub_switch_guard = Some(sentry_core::HubSwitchGuard::new(data.hub.clone())); + let extensions = span.extensions(); + if let Some(data) = extensions.get::() { + let guard = sentry_core::HubSwitchGuard::new(data.hub.clone()); + SPAN_GUARDS.with(|guards| { + guards.borrow_mut().insert(id.clone(), guard); + }); data.hub.configure_scope(|scope| { scope.set_span(Some(data.sentry_span.clone())); - }) + }); } } @@ -366,18 +367,24 @@ where None => return, }; - let mut extensions = span.extensions_mut(); - if let Some(data) = extensions.get_mut::() { + let extensions = span.extensions(); + if let Some(data) = extensions.get::() { data.hub.configure_scope(|scope| { scope.set_span(data.parent_sentry_span.clone()); }); - data.hub_switch_guard.take(); } + SPAN_GUARDS.with(|guards| { + guards.borrow_mut().remove(id); + }); } /// When a span gets closed, finish the underlying sentry span, and set back /// its parent as the *current* sentry span. fn on_close(&self, id: span::Id, ctx: Context<'_, S>) { + SPAN_GUARDS.with(|guards| { + guards.borrow_mut().remove(&id); + }); + let span = match ctx.span(&id) { Some(span) => span, None => return, @@ -503,6 +510,10 @@ fn extract_span_data( thread_local! { static VISITOR_BUFFER: RefCell = const { RefCell::new(String::new()) }; + /// Hub switch guards keyed by span ID. Stored in thread-local so guards are + /// always dropped on the same thread where they were created. + static SPAN_GUARDS: RefCell> = + RefCell::new(HashMap::new()); } /// Records all span fields into a `BTreeMap`, reusing a mutable `String` as buffer. From c77642e4615658b05e12b910fc7086eb480f4f2c Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Wed, 28 Jan 2026 13:59:56 +0100 Subject: [PATCH 2/7] style(tracing): import HubSwitchGuard --- pr-findings.md | 24 ++++++++++++++++++++++++ sentry-tracing/src/layer.rs | 6 +++--- 2 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 pr-findings.md diff --git a/pr-findings.md b/pr-findings.md new file mode 100644 index 000000000..cc1889a3a --- /dev/null +++ b/pr-findings.md @@ -0,0 +1,24 @@ +# PR 957 review findings (updated scope: not targeting #946) + +## PR context +- PR: #957 “fix!(core): Make HubSwitchGuard !Send to prevent thread corruption” (open, base `master`, head `szokeasaurusrex/hubswitchguard`). +- Description says it fixes GitHub issue #943 and Linear RUST-130. +- Commit message still references GitHub issue #946 and Linear RUST-132; those references now appear out of scope and should be removed/updated to match intent. + +## Issue check (in-scope) +### #943 — “HubSwitchGuard should not be Send” (open) +- Repro shows moving a `HubSwitchGuard` across threads corrupts hub state. +- Expected: compile-time error; actual: wrong thread hub is replaced. +- The PR’s `!Send` change in `sentry-core` directly addresses this. + +### RUST-130 (Linear) +- Referenced in PR description; not accessible from GitHub CLI, so I can’t verify details. + +## Are any parts no longer necessary (given #946 is out of scope)? +- The `sentry-tracing` refactor still appears necessary even if #946 is out of scope. Reason: once `HubSwitchGuard` becomes `!Send`, storing it in span extensions (which can be accessed across threads) risks either compile-time Send/Sync issues or runtime drops on the wrong thread. Moving guards to thread-local storage keeps them on the originating thread and avoids making span data `!Send`. +- What does look unnecessary now: the references to #946 / RUST-132 in the commit message and any PR description text implying that fix; those should be removed or adjusted to avoid claiming a second issue as a goal. + +## Files touched (for traceability) +- `CHANGELOG.md`: adds “Unreleased” entry for the breaking change and fix. +- `sentry-core/src/hub_impl.rs`: makes `SwitchGuard`/`HubSwitchGuard` `!Send` using `PhantomData>`. +- `sentry-tracing/src/layer.rs`: removes guard from span extensions; stores it in thread-local map keyed by span ID; removes on exit/close. diff --git a/sentry-tracing/src/layer.rs b/sentry-tracing/src/layer.rs index 72178001b..8c9ddc601 100644 --- a/sentry-tracing/src/layer.rs +++ b/sentry-tracing/src/layer.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use bitflags::bitflags; use sentry_core::protocol::Value; -use sentry_core::{Breadcrumb, TransactionOrSpan}; +use sentry_core::{Breadcrumb, HubSwitchGuard, TransactionOrSpan}; use tracing_core::field::Visit; use tracing_core::{span, Event, Field, Level, Metadata, Subscriber}; use tracing_subscriber::layer::{Context, Layer}; @@ -350,7 +350,7 @@ where let extensions = span.extensions(); if let Some(data) = extensions.get::() { - let guard = sentry_core::HubSwitchGuard::new(data.hub.clone()); + let guard = HubSwitchGuard::new(data.hub.clone()); SPAN_GUARDS.with(|guards| { guards.borrow_mut().insert(id.clone(), guard); }); @@ -512,7 +512,7 @@ thread_local! { static VISITOR_BUFFER: RefCell = const { RefCell::new(String::new()) }; /// Hub switch guards keyed by span ID. Stored in thread-local so guards are /// always dropped on the same thread where they were created. - static SPAN_GUARDS: RefCell> = + static SPAN_GUARDS: RefCell> = RefCell::new(HashMap::new()); } From ab3f7b9318edd11be76890356194fcaa8a798d1a Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Wed, 28 Jan 2026 14:52:48 +0100 Subject: [PATCH 3/7] chore: remove pr-findings doc --- pr-findings.md | 24 ------------------------ 1 file changed, 24 deletions(-) delete mode 100644 pr-findings.md diff --git a/pr-findings.md b/pr-findings.md deleted file mode 100644 index cc1889a3a..000000000 --- a/pr-findings.md +++ /dev/null @@ -1,24 +0,0 @@ -# PR 957 review findings (updated scope: not targeting #946) - -## PR context -- PR: #957 “fix!(core): Make HubSwitchGuard !Send to prevent thread corruption” (open, base `master`, head `szokeasaurusrex/hubswitchguard`). -- Description says it fixes GitHub issue #943 and Linear RUST-130. -- Commit message still references GitHub issue #946 and Linear RUST-132; those references now appear out of scope and should be removed/updated to match intent. - -## Issue check (in-scope) -### #943 — “HubSwitchGuard should not be Send” (open) -- Repro shows moving a `HubSwitchGuard` across threads corrupts hub state. -- Expected: compile-time error; actual: wrong thread hub is replaced. -- The PR’s `!Send` change in `sentry-core` directly addresses this. - -### RUST-130 (Linear) -- Referenced in PR description; not accessible from GitHub CLI, so I can’t verify details. - -## Are any parts no longer necessary (given #946 is out of scope)? -- The `sentry-tracing` refactor still appears necessary even if #946 is out of scope. Reason: once `HubSwitchGuard` becomes `!Send`, storing it in span extensions (which can be accessed across threads) risks either compile-time Send/Sync issues or runtime drops on the wrong thread. Moving guards to thread-local storage keeps them on the originating thread and avoids making span data `!Send`. -- What does look unnecessary now: the references to #946 / RUST-132 in the commit message and any PR description text implying that fix; those should be removed or adjusted to avoid claiming a second issue as a goal. - -## Files touched (for traceability) -- `CHANGELOG.md`: adds “Unreleased” entry for the breaking change and fix. -- `sentry-core/src/hub_impl.rs`: makes `SwitchGuard`/`HubSwitchGuard` `!Send` using `PhantomData>`. -- `sentry-tracing/src/layer.rs`: removes guard from span extensions; stores it in thread-local map keyed by span ID; removes on exit/close. From ddf0e1fe9fa460818b56a1073c60ea8309a04d6e Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Fri, 30 Jan 2026 15:04:10 +0100 Subject: [PATCH 4/7] fix(tracing): drop hub guard before span lookup --- sentry-tracing/src/layer.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sentry-tracing/src/layer.rs b/sentry-tracing/src/layer.rs index 8c9ddc601..d722b472b 100644 --- a/sentry-tracing/src/layer.rs +++ b/sentry-tracing/src/layer.rs @@ -362,6 +362,8 @@ where /// Set exited span's parent as *current* sentry span. fn on_exit(&self, id: &span::Id, ctx: Context<'_, S>) { + let _guard = SPAN_GUARDS.with(|guards| guards.borrow_mut().remove(id)); + let span = match ctx.span(id) { Some(span) => span, None => return, @@ -373,9 +375,6 @@ where scope.set_span(data.parent_sentry_span.clone()); }); } - SPAN_GUARDS.with(|guards| { - guards.borrow_mut().remove(id); - }); } /// When a span gets closed, finish the underlying sentry span, and set back From 6169b08253035efbc4107ae554e17813e035be28 Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Thu, 5 Feb 2026 14:57:25 +0100 Subject: [PATCH 5/7] fix issue #946 --- sentry-tracing/src/{layer.rs => layer/mod.rs} | 19 +-- sentry-tracing/src/layer/span_guard_stack.rs | 118 ++++++++++++++++++ sentry-tracing/tests/span_cross_thread.rs | 77 ++++++++++++ sentry-tracing/tests/span_reentrancy.rs | 70 +++++++++++ 4 files changed, 276 insertions(+), 8 deletions(-) rename sentry-tracing/src/{layer.rs => layer/mod.rs} (97%) create mode 100644 sentry-tracing/src/layer/span_guard_stack.rs create mode 100644 sentry-tracing/tests/span_cross_thread.rs create mode 100644 sentry-tracing/tests/span_reentrancy.rs diff --git a/sentry-tracing/src/layer.rs b/sentry-tracing/src/layer/mod.rs similarity index 97% rename from sentry-tracing/src/layer.rs rename to sentry-tracing/src/layer/mod.rs index d722b472b..5e9848968 100644 --- a/sentry-tracing/src/layer.rs +++ b/sentry-tracing/src/layer/mod.rs @@ -1,11 +1,11 @@ use std::borrow::Cow; use std::cell::RefCell; -use std::collections::{BTreeMap, HashMap}; +use std::collections::BTreeMap; use std::sync::Arc; use bitflags::bitflags; use sentry_core::protocol::Value; -use sentry_core::{Breadcrumb, HubSwitchGuard, TransactionOrSpan}; +use sentry_core::{Breadcrumb, Hub, HubSwitchGuard, TransactionOrSpan}; use tracing_core::field::Visit; use tracing_core::{span, Event, Field, Level, Metadata, Subscriber}; use tracing_subscriber::layer::{Context, Layer}; @@ -16,6 +16,9 @@ use crate::SENTRY_NAME_FIELD; use crate::SENTRY_OP_FIELD; use crate::SENTRY_TRACE_FIELD; use crate::TAGS_PREFIX; +use span_guard_stack::SpanGuardStack; + +mod span_guard_stack; bitflags! { /// The action that Sentry should perform for a given [`Event`] @@ -350,11 +353,12 @@ where let extensions = span.extensions(); if let Some(data) = extensions.get::() { - let guard = HubSwitchGuard::new(data.hub.clone()); + let hub = Arc::new(Hub::new_from_top(data.hub.clone())); + let guard = HubSwitchGuard::new(hub.clone()); SPAN_GUARDS.with(|guards| { - guards.borrow_mut().insert(id.clone(), guard); + guards.borrow_mut().push(id.clone(), guard); }); - data.hub.configure_scope(|scope| { + hub.configure_scope(|scope| { scope.set_span(Some(data.sentry_span.clone())); }); } @@ -362,7 +366,7 @@ where /// Set exited span's parent as *current* sentry span. fn on_exit(&self, id: &span::Id, ctx: Context<'_, S>) { - let _guard = SPAN_GUARDS.with(|guards| guards.borrow_mut().remove(id)); + let _guard = SPAN_GUARDS.with(|guards| guards.borrow_mut().pop(id.clone())); let span = match ctx.span(id) { Some(span) => span, @@ -511,8 +515,7 @@ thread_local! { static VISITOR_BUFFER: RefCell = const { RefCell::new(String::new()) }; /// Hub switch guards keyed by span ID. Stored in thread-local so guards are /// always dropped on the same thread where they were created. - static SPAN_GUARDS: RefCell> = - RefCell::new(HashMap::new()); + static SPAN_GUARDS: RefCell = RefCell::new(SpanGuardStack::new()); } /// Records all span fields into a `BTreeMap`, reusing a mutable `String` as buffer. diff --git a/sentry-tracing/src/layer/span_guard_stack.rs b/sentry-tracing/src/layer/span_guard_stack.rs new file mode 100644 index 000000000..e18435290 --- /dev/null +++ b/sentry-tracing/src/layer/span_guard_stack.rs @@ -0,0 +1,118 @@ +//! This module contains code for stack-like storage for `HubSwitchGuard`s keyed +//! by tracing span ID. + +use std::collections::hash_map::Entry; +use std::collections::HashMap; + +use sentry_core::HubSwitchGuard; +use tracing_core::span::Id as SpanId; + +/// Holds per-span stacks of `HubSwitchGuard`s to handle span re-entrancy. +/// +/// Each time a span is entered, we should push a new guard onto the stack. +/// When the span exits, we should pop the guard from the stack. +pub(super) struct SpanGuardStack { + /// The map of span IDs to their respective guard stacks. + guards: HashMap>, +} + +impl SpanGuardStack { + /// Creates an empty guard stack map. + pub(super) fn new() -> Self { + Self { + guards: HashMap::new(), + } + } + + /// Pushes a guard for the given span ID, creating the stack if needed. + pub(super) fn push(&mut self, id: SpanId, guard: HubSwitchGuard) { + self.guards.entry(id).or_default().push(guard); + } + + /// Pops the most recent guard for the span ID, removing the stack when empty. + pub(super) fn pop(&mut self, id: SpanId) -> Option { + match self.guards.entry(id) { + Entry::Occupied(mut entry) => { + let stack = entry.get_mut(); + let guard = stack.pop(); + if stack.is_empty() { + entry.remove(); + } + guard + } + Entry::Vacant(_) => None, + } + } + + /// Removes all guards for the span ID without returning them. + /// + /// This function guarantees that the guards are dropped in LIFO order. + /// That way, the hub which was active when the span was first entered + /// will be the one active after this function returns. + /// + /// Typically, remove should only get called once the span is fully + /// exited, so this removal order guarantee is mostly just defensive. + pub(super) fn remove(&mut self, id: &SpanId) { + self.guards + .remove(id) + .into_iter() + .flatten() + .rev() // <- we drop in reverse order + .for_each(drop); + } +} + +#[cfg(test)] +mod tests { + use super::SpanGuardStack; + use sentry_core::{Hub, HubSwitchGuard}; + use std::sync::Arc; + use tracing_core::span::Id as SpanId; + + #[test] + fn pop_is_lifo() { + let initial = Hub::current(); + let hub_a = Arc::new(Hub::new_from_top(initial.clone())); + let hub_b = Arc::new(Hub::new_from_top(hub_a.clone())); + + let mut stack = SpanGuardStack::new(); + let id = SpanId::from_u64(1); + + stack.push(id.clone(), HubSwitchGuard::new(hub_a.clone())); + assert!(Arc::ptr_eq(&Hub::current(), &hub_a)); + + stack.push(id.clone(), HubSwitchGuard::new(hub_b.clone())); + assert!(Arc::ptr_eq(&Hub::current(), &hub_b)); + + drop(stack.pop(id.clone()).expect("guard for hub_b")); + assert!(Arc::ptr_eq(&Hub::current(), &hub_a)); + + drop(stack.pop(id.clone()).expect("guard for hub_a")); + assert!(Arc::ptr_eq(&Hub::current(), &initial)); + + assert!(stack.pop(id).is_none()); + } + + #[test] + fn remove_drops_all_guards_in_lifo_order() { + let initial = Hub::current(); + let hub_a = Arc::new(Hub::new_from_top(initial.clone())); + let hub_b = Arc::new(Hub::new_from_top(hub_a.clone())); + + assert!(!Arc::ptr_eq(&hub_b, &initial)); + assert!(!Arc::ptr_eq(&hub_a, &initial)); + assert!(!Arc::ptr_eq(&hub_a, &hub_b)); + + let mut stack = SpanGuardStack::new(); + let id = SpanId::from_u64(2); + + stack.push(id.clone(), HubSwitchGuard::new(hub_a.clone())); + assert!(Arc::ptr_eq(&Hub::current(), &hub_a)); + + stack.push(id.clone(), HubSwitchGuard::new(hub_b.clone())); + assert!(Arc::ptr_eq(&Hub::current(), &hub_b)); + + stack.remove(&id); + assert!(Arc::ptr_eq(&Hub::current(), &initial)); + } +} diff --git a/sentry-tracing/tests/span_cross_thread.rs b/sentry-tracing/tests/span_cross_thread.rs new file mode 100644 index 000000000..5eaa2cdd3 --- /dev/null +++ b/sentry-tracing/tests/span_cross_thread.rs @@ -0,0 +1,77 @@ +mod shared; + +use sentry::protocol::Context; +use std::thread; +use std::time::Duration; + +#[test] +fn cross_thread_span_entries_share_transaction() { + let transport = shared::init_sentry(1.0); + + let span = tracing::info_span!("foo"); + let span2 = span.clone(); + + let handle1 = thread::spawn(move || { + let _guard = span.enter(); + let _bar_span = tracing::info_span!("bar").entered(); + thread::sleep(Duration::from_millis(100)); + }); + + let handle2 = thread::spawn(move || { + thread::sleep(Duration::from_millis(10)); + let _guard = span2.enter(); + let _baz_span = tracing::info_span!("baz").entered(); + thread::sleep(Duration::from_millis(50)); + }); + + handle1.join().unwrap(); + handle2.join().unwrap(); + + let data = transport.fetch_and_clear_envelopes(); + let transactions: Vec<_> = data + .into_iter() + .flat_map(|envelope| { + envelope + .items() + .filter_map(|item| match item { + sentry::protocol::EnvelopeItem::Transaction(transaction) => { + Some(transaction.clone()) + } + _ => None, + }) + .collect::>() + }) + .collect(); + + assert_eq!( + transactions.len(), + 1, + "expected a single transaction for cross-thread span entries" + ); + + let transaction = &transactions[0]; + assert_eq!(transaction.name.as_deref(), Some("foo")); + + let trace = match transaction + .contexts + .get("trace") + .expect("transaction should include trace context") + { + Context::Trace(trace) => trace, + unexpected => panic!("expected trace context but got {unexpected:?}"), + }; + + let bar_span = transaction + .spans + .iter() + .find(|span| span.description.as_deref() == Some("bar")) + .expect("expected span \"bar\" to be recorded in the transaction"); + let baz_span = transaction + .spans + .iter() + .find(|span| span.description.as_deref() == Some("baz")) + .expect("expected span \"baz\" to be recorded in the transaction"); + + assert_eq!(bar_span.parent_span_id, Some(trace.span_id)); + assert_eq!(baz_span.parent_span_id, Some(trace.span_id)); +} diff --git a/sentry-tracing/tests/span_reentrancy.rs b/sentry-tracing/tests/span_reentrancy.rs new file mode 100644 index 000000000..8fc55aad4 --- /dev/null +++ b/sentry-tracing/tests/span_reentrancy.rs @@ -0,0 +1,70 @@ +use sentry::protocol::EnvelopeItem; + +mod shared; + +/// Ensures re-entering the same span does not corrupt the current tracing state, +/// so subsequent spans are still recorded under a single transaction. +#[test] +fn reentering_span_preserves_parent() { + let transport = shared::init_sentry(1.0); + + { + // Create a span and enter it, then re-enter the same span to simulate + // common async polling behavior where a span can be entered multiple times. + let span_a = tracing::info_span!("a"); + let _enter_a = span_a.enter(); + { + let _reenter_a = span_a.enter(); + } + + // Create another span while the original span is still entered to ensure + // it is recorded on the same transaction rather than starting a new one. + let span_b = tracing::info_span!("b"); + { + let _enter_b = span_b.enter(); + } + } + + let transactions: Vec<_> = transport + .fetch_and_clear_envelopes() + .into_iter() + .flat_map(|envelope| envelope.into_items()) + .filter_map(|item| match item { + EnvelopeItem::Transaction(transaction) => Some(transaction), + _ => None, + }) + .collect(); + + assert_eq!( + transactions.len(), + 1, + "expected a single transaction when reentering a span" + ); + + let transaction = &transactions[0]; + assert_eq!(transaction.name.as_deref(), Some("a")); + + let trace = match transaction + .contexts + .get("trace") + .expect("transaction should include trace context") + { + sentry::protocol::Context::Trace(trace) => trace, + unexpected => panic!("expected trace context but got {unexpected:?}"), + }; + + let b_span = transaction + .spans + .iter() + .find(|span| span.description.as_deref() == Some("b")) + .expect("expected span \"b\" to be recorded in the transaction"); + + assert_eq!(b_span.parent_span_id, Some(trace.span_id)); + assert!( + !transaction + .spans + .iter() + .any(|span| span.description.as_deref() == Some("a")), + "expected the transaction root span not to be duplicated in span list" + ); +} From 4e1c529a2c051ccee2b82340f1d30260dfbdde0c Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Thu, 5 Feb 2026 15:27:26 +0100 Subject: [PATCH 6/7] avoid unneeded clones --- sentry-tracing/src/layer/mod.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/sentry-tracing/src/layer/mod.rs b/sentry-tracing/src/layer/mod.rs index 5e9848968..a130fe4f7 100644 --- a/sentry-tracing/src/layer/mod.rs +++ b/sentry-tracing/src/layer/mod.rs @@ -353,14 +353,17 @@ where let extensions = span.extensions(); if let Some(data) = extensions.get::() { - let hub = Arc::new(Hub::new_from_top(data.hub.clone())); - let guard = HubSwitchGuard::new(hub.clone()); - SPAN_GUARDS.with(|guards| { - guards.borrow_mut().push(id.clone(), guard); - }); + let hub = Arc::new(Hub::new_from_top(&data.hub)); + hub.configure_scope(|scope| { scope.set_span(Some(data.sentry_span.clone())); }); + + let guard = HubSwitchGuard::new(hub); + + SPAN_GUARDS.with(|guards| { + guards.borrow_mut().push(id.clone(), guard); + }); } } From 96bcd82512116506955872f37ef0dd2c3d00bd07 Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Thu, 5 Feb 2026 15:44:10 +0100 Subject: [PATCH 7/7] reorganize, add some comments --- sentry-tracing/src/layer/mod.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/sentry-tracing/src/layer/mod.rs b/sentry-tracing/src/layer/mod.rs index a130fe4f7..63b5e7242 100644 --- a/sentry-tracing/src/layer/mod.rs +++ b/sentry-tracing/src/layer/mod.rs @@ -353,6 +353,14 @@ where let extensions = span.extensions(); if let Some(data) = extensions.get::() { + // We fork the hub (based on the hub associated with the span) + // upon entering the span. This prevents data leakage if the span + // is entered and exited multiple times. + // + // Further, Hubs are meant to manage thread-local state, even + // though they can be shared across threads. As the span may being + // entered on a different thread than where it was created, we need + // to use a new hub to avoid altering state on the original thread. let hub = Arc::new(Hub::new_from_top(&data.hub)); hub.configure_scope(|scope| { @@ -369,8 +377,6 @@ where /// Set exited span's parent as *current* sentry span. fn on_exit(&self, id: &span::Id, ctx: Context<'_, S>) { - let _guard = SPAN_GUARDS.with(|guards| guards.borrow_mut().pop(id.clone())); - let span = match ctx.span(id) { Some(span) => span, None => return, @@ -382,11 +388,19 @@ where scope.set_span(data.parent_sentry_span.clone()); }); } + + // Drop the guard to switch back to previous hub + SPAN_GUARDS.with(|guards| guards.borrow_mut().pop(id.clone())); } /// When a span gets closed, finish the underlying sentry span, and set back /// its parent as the *current* sentry span. fn on_close(&self, id: span::Id, ctx: Context<'_, S>) { + // Ensure all remaining Hub guards are dropped, to restore the original + // Hub. + // + // By this point, the span probably should be fully executed, but we should + // still ensure the guard is dropped in case this expectation is violated. SPAN_GUARDS.with(|guards| { guards.borrow_mut().remove(&id); });