From d0f44d80b4d85b918edd0d32393e12c40a1c28ff Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Wed, 14 Jan 2026 13:48:32 +0100 Subject: [PATCH 01/10] fix!(core): Make HubSwitchGuard !Send to prevent thread corruption (#957) ## Description This PR does three important things, which are all interdependent, so I think it makes sense to do it all in a single PR. ### (1) Making `HubSwitchGuard` `!Send` This PR makes `HubSwitchGuard` `!Send` by adding `PhantomData>` while keeping it `Sync`. The type system now prevents the guard from being moved across threads at compile time. This change is important because `HubSwitchGuard` manages thread-local hub state, but was previously `Send`, allowing it to be moved to another thread. When dropped on the wrong thread, it could corrupt that thread's hub state instead of restoring the original thread. This change resolves #943. **Tests related to this change:** `!Send` is enforced by the compiler, so there are no additional tests here. ### (2) A new stack for spans to manage `HubSwitchGuards` As `HubSwitchGuard` is now `!Send`, we needed a new way to manage the `HubSwitchGuard` associated with a given span, in the `tracing` integration. Previously, we just had the guards live directly on the span via the `SentrySpanData` type, but this no longer works, since spans need to be `Send`. So, we now declare a thread-local mapping from span IDs to `HubSwitchGuard`s. The guards are stored in a stack, as each span will have one guard per span entry (spans can be entered multiple times, even on the same thread, and without the stack, we would restore the original hub too early). We drop the guards on span exit in LIFO order. **Tests related to this change:** [`span_reentrancy.rs`](https://github.com/getsentry/sentry-rust/pull/957/changes#diff-203b34b6e9f8f4bad9c8c43e05c781e119eea7b27648f257bfe7b35e912ba2b1) contains tests to validate correct span tree structure when re-entering the same span multiple times. A previous iteration of this PR only allowed a single guard per span; the test failed against this implementation because it produces an incorrect span structure (two transactions instead of one). The test only passes with the guard stack. ### (3) Forking the `Hub` on each span (re-)entry Change (2) is insufficient to make the [`span_reentrancy.rs`](https://github.com/getsentry/sentry-rust/pull/957/changes#diff-203b34b6e9f8f4bad9c8c43e05c781e119eea7b27648f257bfe7b35e912ba2b1) test pass because with only that change, there is still the fundamental problem that each span does not get its own `Hub` to manage state. Thus, in that test, I believe we were actually only ever using one hub, because there was no place we were forking the hub. So, on span exit, we prematurely [set the parent span on the hub](https://github.com/getsentry/sentry-rust/pull/957/changes#diff-5acb70e20dc764b608e1acf81b57fea59308624b7c2bc87906b310ff8b1f0eb2L372). Forking the hub ensures proper isolation, so the span gets set back at the right time (I also [suspect](https://github.com/getsentry/sentry-rust/pull/957/changes#r2773227449) we don't need to manually set it back, but I am unsure). This change resolves #946. **Tests related to this change:** [`span_cross_thread.rs`](https://github.com/getsentry/sentry-rust/pull/957/changes#diff-aa629e96442a0995ed2fd39dd68a18dd4be293732d2435b9aa5e03c848e12c38) ensures that entering the same span in multiple threads produces the correct span tree. Basically, it is a reproduction of #946. [`span_reentrancy.rs`](https://github.com/getsentry/sentry-rust/pull/957/changes#diff-203b34b6e9f8f4bad9c8c43e05c781e119eea7b27648f257bfe7b35e912ba2b1) also only passes with this change. ## Issues - Fixes #943 - Fixes [RUST-130](https://linear.app/getsentry/issue/RUST-130/hubswitchguard-should-not-be-send) - Fixes #946 - Fixes [RUST-132](https://linear.app/getsentry/issue/RUST-132/entering-the-same-span-several-times-causes-esoteric-behavior) --- CHANGELOG.md | 5 + sentry-core/src/hub_impl.rs | 16 ++- sentry-tracing/src/{layer.rs => layer/mod.rs} | 66 ++++++---- sentry-tracing/src/layer/span_guard_stack.rs | 118 ++++++++++++++++++ sentry-tracing/tests/span_cross_thread.rs | 77 ++++++++++++ sentry-tracing/tests/span_reentrancy.rs | 70 +++++++++++ 6 files changed, 321 insertions(+), 31 deletions(-) rename sentry-tracing/src/{layer.rs => layer/mod.rs} (90%) 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/CHANGELOG.md b/CHANGELOG.md index f5abb9850..53d046382 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,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)). - Expose transport utilities ([#949](https://github.com/getsentry/sentry-rust/pull/949)) +### 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/mod.rs similarity index 90% rename from sentry-tracing/src/layer.rs rename to sentry-tracing/src/layer/mod.rs index d02c0496d..47a85d26c 100644 --- a/sentry-tracing/src/layer.rs +++ b/sentry-tracing/src/layer/mod.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, 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`] @@ -234,9 +237,7 @@ fn record_fields<'a, K: AsRef + Into>>( /// the *current* span. pub(super) struct SentrySpanData { pub(super) sentry_span: TransactionOrSpan, - parent_sentry_span: Option, hub: Arc, - hub_switch_guard: Option, } impl Layer for SentryLayer @@ -334,12 +335,7 @@ where set_default_attributes(&mut sentry_span, span.metadata()); let mut extensions = span.extensions_mut(); - extensions.insert(SentrySpanData { - sentry_span, - parent_sentry_span, - hub, - hub_switch_guard: None, - }); + extensions.insert(SentrySpanData { sentry_span, hub }); } /// Sets entered span as *current* sentry span. A tracing span can be @@ -350,34 +346,47 @@ 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())); - data.hub.configure_scope(|scope| { + 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| { scope.set_span(Some(data.sentry_span.clone())); - }) - } - } + }); - /// Set exited span's parent as *current* sentry span. - fn on_exit(&self, id: &span::Id, ctx: Context<'_, S>) { - let span = match ctx.span(id) { - Some(span) => span, - None => return, - }; + let guard = HubSwitchGuard::new(hub); - let mut extensions = span.extensions_mut(); - if let Some(data) = extensions.get_mut::() { - data.hub.configure_scope(|scope| { - scope.set_span(data.parent_sentry_span.clone()); + SPAN_GUARDS.with(|guards| { + guards.borrow_mut().push(id.clone(), guard); }); - data.hub_switch_guard.take(); } } + /// Drop the current span's [`HubSwitchGuard`] to restore the parent [`Hub`]. + fn on_exit(&self, id: &span::Id, _: Context<'_, S>) { + 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); + }); + let span = match ctx.span(&id) { Some(span) => span, None => return, @@ -503,6 +512,9 @@ 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(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 ce0b7b3a71fc722f38f46e3963c6e9e1d2afa152 Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Thu, 26 Feb 2026 14:33:53 +0100 Subject: [PATCH 02/10] fixup! logging for span exited on other thread --- sentry-core/src/macros.rs | 51 +++++++ sentry-tracing/src/layer/mod.rs | 38 +++-- sentry-tracing/src/layer/span_guard_stack.rs | 41 +---- .../tests/await_cross_thread_guard.rs | 144 ++++++++++++++++++ 4 files changed, 220 insertions(+), 54 deletions(-) create mode 100644 sentry-tracing/tests/await_cross_thread_guard.rs diff --git a/sentry-core/src/macros.rs b/sentry-core/src/macros.rs index edb75dcd5..5dcdc78cd 100644 --- a/sentry-core/src/macros.rs +++ b/sentry-core/src/macros.rs @@ -64,6 +64,37 @@ macro_rules! sentry_debug { } } +/// Panics in debug builds and logs through `sentry_debug!` in non-debug builds. +#[macro_export] +#[doc(hidden)] +macro_rules! debug_panic_or_log { + ($($arg:tt)*) => {{ + #[cfg(debug_assertions)] + panic!($($arg)*); + + #[cfg(not(debug_assertions))] + $crate::sentry_debug!($($arg)*); + }}; +} + +/// If the condition is false, panics in debug builds and logs in non-debug builds. +#[macro_export] +#[doc(hidden)] +macro_rules! debug_assert_or_log { + ($cond:expr $(,)?) => {{ + let condition = $cond; + if !condition { + $crate::debug_panic_or_log!("assertion failed: {}", stringify!($cond)); + } + }}; + ($cond:expr, $($arg:tt)+) => {{ + let condition = $cond; + if !condition { + $crate::debug_panic_or_log!($($arg)+); + } + }}; +} + #[allow(unused_macros)] macro_rules! minimal_unreachable { () => { @@ -73,3 +104,23 @@ macro_rules! minimal_unreachable { ); }; } + +#[cfg(test)] +mod tests { + #[test] + fn debug_assert_or_log_does_not_panic_when_condition_holds() { + crate::debug_assert_or_log!(2 + 2 == 4, "should not panic"); + } + + #[test] + #[should_panic(expected = "assertion failed: 1 == 2")] + fn debug_assert_or_log_panics_with_default_message_when_condition_fails() { + crate::debug_assert_or_log!(1 == 2); + } + + #[test] + #[should_panic(expected = "custom invariant message")] + fn debug_assert_or_log_panics_with_custom_message_when_condition_fails() { + crate::debug_assert_or_log!(false, "custom invariant message"); + } +} diff --git a/sentry-tracing/src/layer/mod.rs b/sentry-tracing/src/layer/mod.rs index 47a85d26c..e7f59509a 100644 --- a/sentry-tracing/src/layer/mod.rs +++ b/sentry-tracing/src/layer/mod.rs @@ -338,8 +338,16 @@ where extensions.insert(SentrySpanData { sentry_span, hub }); } - /// Sets entered span as *current* sentry span. A tracing span can be - /// entered and existed multiple times, for example, when using a `tracing::Instrumented` future. + /// Sets the entered span as *current* sentry span. + /// + /// A tracing span can be entered and exited multiple times, for example, + /// when using a `tracing::Instrumented` future. + /// + /// Spans must be exited on the same span that they are entered. The + /// `sentry-tracing` integration's behavior is undefined if spans are + /// exited on threads other than the one they are entered from; + /// specifically, doing so will likely cause data to bleed between + /// [`Hub`]s in unexpected ways. fn on_enter(&self, id: &span::Id, ctx: Context<'_, S>) { let span = match ctx.span(id) { Some(span) => span, @@ -372,21 +380,20 @@ where /// Drop the current span's [`HubSwitchGuard`] to restore the parent [`Hub`]. fn on_exit(&self, id: &span::Id, _: Context<'_, S>) { - SPAN_GUARDS.with(|guards| guards.borrow_mut().pop(id.clone())); + let popped = SPAN_GUARDS.with(|guards| guards.borrow_mut().pop(id.clone())); + + sentry_core::debug_assert_or_log!( + popped.is_some(), + "[SentryLayer] missing HubSwitchGuard on exit for span {id:?}; \ + expected balanced enter/exit on the same thread. Likely causes: cross-thread exit, \ + unbalanced enter/exit, or holding enter guards across `.await` \ + (prefer `Span::in_scope`/`Future::instrument`)." + ); } /// 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); - }); - let span = match ctx.span(&id) { Some(span) => span, None => return, @@ -512,8 +519,11 @@ 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. + /// Hub switch guards keyed by span ID. + /// + /// Guard bookkeeping is thread-local by design. Correctness expects + /// balanced enter/exit callbacks on the same thread; `on_close` performs + /// best-effort fallback cleanup for mismatches. static SPAN_GUARDS: RefCell = RefCell::new(SpanGuardStack::new()); } diff --git a/sentry-tracing/src/layer/span_guard_stack.rs b/sentry-tracing/src/layer/span_guard_stack.rs index e18435290..753454a27 100644 --- a/sentry-tracing/src/layer/span_guard_stack.rs +++ b/sentry-tracing/src/layer/span_guard_stack.rs @@ -30,6 +30,7 @@ impl SpanGuardStack { } /// Pops the most recent guard for the span ID, removing the stack when empty. + #[must_use] pub(super) fn pop(&mut self, id: SpanId) -> Option { match self.guards.entry(id) { Entry::Occupied(mut entry) => { @@ -43,23 +44,6 @@ impl SpanGuardStack { 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)] @@ -92,27 +76,4 @@ mod tests { 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/await_cross_thread_guard.rs b/sentry-tracing/tests/await_cross_thread_guard.rs new file mode 100644 index 000000000..5025294fd --- /dev/null +++ b/sentry-tracing/tests/await_cross_thread_guard.rs @@ -0,0 +1,144 @@ +mod shared; + +use std::future::{self, Future}; +use std::sync::mpsc; +use std::task::{Context, Poll, Waker}; +use std::thread; + +use sentry::protocol::{EnvelopeItem, Transaction}; +use sentry::Envelope; +use tracing::Level; + +/// Test that the [`sentry_tracing::SentryLayer`]'s `on_exit` implementation panics +/// (only when `debug_assertions` are enabled) if a span is exited on a span that +/// was entered on a different thread than where it was exited. Here, we specifically +/// test a future that is awaited across threads, as that is probably the most common +/// scenario where this can occur. +#[test] +fn future_cross_thread() { + let transport = shared::init_sentry(1.0); + + let mut future = Box::pin(span_across_await()); + + let (tx, rx) = mpsc::channel(); + + let thread1 = thread::spawn(move || { + let poll = future.as_mut().poll(&mut noop_context()); + assert!(poll.is_pending(), "future should be pending"); + tx.send(future).expect("failed to send future"); + }); + + let thread2 = thread::spawn(move || { + let _ = rx + .recv() + .expect("failed to receive future") + .as_mut() + .poll(&mut noop_context()); + }); + + thread1 + .join() + .expect("thread 1 panicked, but should have completed"); + + let thread2_panic_message = thread2 + .join() + .expect_err("thread2 did not panic, but it should have") + .downcast::() + .expect("expected thread2 to panic with a String message"); + + assert!( + thread2_panic_message.starts_with("[SentryLayer] missing HubSwitchGuard on exit for span"), + "Thread 2's panicked, but not for the expected reason. It is also possible that the panic \ + message was changed without updating this test." + ); + + // Despite the panic, the transaction should still get sent to Sentry + assert_transaction(transport.fetch_and_clear_envelopes()); +} + +/// Counterpart to [`future_cross_thread`]; here, we check that the panic asserted in +/// [`future_cross_thread`] is not triggered when the span is exited on the same thread +/// that it was entered on. +#[test] +fn futures_same_thread() { + let transport = shared::init_sentry(1.0); + + let mut future = Box::pin(span_across_await()); + + let thread = thread::spawn(move || { + assert!( + future.as_mut().poll(&mut noop_context()).is_pending(), + "first poll should be pending" + ); + assert!( + future.as_mut().poll(&mut noop_context()).is_ready(), + "second poll should be ready" + ); + }); + + thread.join().expect("thread should complete successfully"); + assert_transaction(transport.fetch_and_clear_envelopes()); +} + +/// Assert that the given envelopes contain exactly one [`Envelope`], +/// containing exactly one [`EnvelopeItem`], which is a [`Transaction`], +/// with [`name`](Transaction::name) `"span_across_await"`. +fn assert_transaction(envelopes: Vec) { + let envelope = get_and_assert_only_item(envelopes, "expected exactly one envelope"); + let item = get_and_assert_only_item(envelope.into_items(), "expected exactly one item"); + + assert!( + matches!( + item, + EnvelopeItem::Transaction(Transaction { + name: Some(expected_name), + .. + }) if expected_name == "span_across_await" + ), + "expected a Transaction item with name \"span_across_await\"" + ); +} + +/// A helper function which creates and [`enter`s](tracing::Span::enter) +/// a [`Span`](tracing::Span), holding the returned +/// [`Entered<'_>`](tracing::span::Entered) guard across an `.await` boundary. +async fn span_across_await() { + let span = tracing::span!(Level::INFO, "span_across_await"); + let _entered = span.enter(); + + yield_once().await; + // _entered dropped here, after .await call +} + +/// Helper function that yields exactly once, then finishes. +async fn yield_once() { + let mut yielded = false; + + future::poll_fn(|_| { + if yielded { + Poll::Ready(()) + } else { + yielded = true; + Poll::Pending + } + }) + .await; +} + +/// Helper function to create a new no-op [`Context`]. +fn noop_context() -> Context<'static> { + Context::from_waker(Waker::noop()) +} + +/// Helper function to get and assert that there is exactly one item in the iterator. +/// Extracts the only item from the iterator and returns it, or panics with the +/// provided message if there are zero or multiple items. +fn get_and_assert_only_item(item_iter: I, message: &str) -> I::Item +where + I: IntoIterator, +{ + let mut iter = item_iter.into_iter(); + let item = iter.next().expect(message); + assert!(iter.next().is_none(), "{message}"); + item +} From 9ef8d5fb4142716cc1dc14d5e76c1c1140ee9efa Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Thu, 26 Feb 2026 15:41:25 +0100 Subject: [PATCH 03/10] fixup! Concurrency issue --- sentry-tracing/tests/await_cross_thread_guard.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sentry-tracing/tests/await_cross_thread_guard.rs b/sentry-tracing/tests/await_cross_thread_guard.rs index 5025294fd..7a3b40e1a 100644 --- a/sentry-tracing/tests/await_cross_thread_guard.rs +++ b/sentry-tracing/tests/await_cross_thread_guard.rs @@ -6,8 +6,8 @@ use std::task::{Context, Poll, Waker}; use std::thread; use sentry::protocol::{EnvelopeItem, Transaction}; -use sentry::Envelope; -use tracing::Level; +use sentry::{Envelope, Hub, HubSwitchGuard}; +use tracing::Span; /// Test that the [`sentry_tracing::SentryLayer`]'s `on_exit` implementation panics /// (only when `debug_assertions` are enabled) if a span is exited on a span that @@ -16,6 +16,7 @@ use tracing::Level; /// scenario where this can occur. #[test] fn future_cross_thread() { + let _guard = HubSwitchGuard::new(Hub::new_from_top(Hub::current()).into()); let transport = shared::init_sentry(1.0); let mut future = Box::pin(span_across_await()); @@ -61,6 +62,7 @@ fn future_cross_thread() { /// that it was entered on. #[test] fn futures_same_thread() { + let _guard = HubSwitchGuard::new(Hub::new_from_top(Hub::current()).into()); let transport = shared::init_sentry(1.0); let mut future = Box::pin(span_across_await()); From 6c1fe0be42e32c298ffba20d6b7b60f25a2d4f68 Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Thu, 26 Feb 2026 16:45:35 +0100 Subject: [PATCH 04/10] fixup! only expect pop for Sentry spans --- sentry-tracing/src/layer/mod.rs | 19 +-- .../tests/await_cross_thread_guard.rs | 127 ++++++++++++------ 2 files changed, 95 insertions(+), 51 deletions(-) diff --git a/sentry-tracing/src/layer/mod.rs b/sentry-tracing/src/layer/mod.rs index e7f59509a..ed6dee496 100644 --- a/sentry-tracing/src/layer/mod.rs +++ b/sentry-tracing/src/layer/mod.rs @@ -379,15 +379,19 @@ where } /// Drop the current span's [`HubSwitchGuard`] to restore the parent [`Hub`]. - fn on_exit(&self, id: &span::Id, _: Context<'_, S>) { + fn on_exit(&self, id: &span::Id, ctx: Context<'_, S>) { let popped = SPAN_GUARDS.with(|guards| guards.borrow_mut().pop(id.clone())); + // We should have popped a guard if the tracing span has `SentrySpanData` extensions. sentry_core::debug_assert_or_log!( - popped.is_some(), - "[SentryLayer] missing HubSwitchGuard on exit for span {id:?}; \ - expected balanced enter/exit on the same thread. Likely causes: cross-thread exit, \ - unbalanced enter/exit, or holding enter guards across `.await` \ - (prefer `Span::in_scope`/`Future::instrument`)." + popped.is_some() + || ctx + .span(id) + .is_none_or(|span| span.extensions().get::().is_none()), + "[SentryLayer] missing HubSwitchGuard on exit for span {id:?}. \ + This span has been exited more times on this thread than it has been entered, \ + likely due to dropping an `Entered` guard in a different thread than where it was \ + entered. This mismatch will likely cause the sentry-tracing layer to leak memory." ); } @@ -522,8 +526,7 @@ thread_local! { /// Hub switch guards keyed by span ID. /// /// Guard bookkeeping is thread-local by design. Correctness expects - /// balanced enter/exit callbacks on the same thread; `on_close` performs - /// best-effort fallback cleanup for mismatches. + /// balanced enter/exit callbacks on the same thread. static SPAN_GUARDS: RefCell = RefCell::new(SpanGuardStack::new()); } diff --git a/sentry-tracing/tests/await_cross_thread_guard.rs b/sentry-tracing/tests/await_cross_thread_guard.rs index 7a3b40e1a..05807dd52 100644 --- a/sentry-tracing/tests/await_cross_thread_guard.rs +++ b/sentry-tracing/tests/await_cross_thread_guard.rs @@ -1,5 +1,6 @@ mod shared; +use std::any::Any; use std::future::{self, Future}; use std::sync::mpsc; use std::task::{Context, Poll, Waker}; @@ -10,62 +11,69 @@ use sentry::{Envelope, Hub, HubSwitchGuard}; use tracing::Span; /// Test that the [`sentry_tracing::SentryLayer`]'s `on_exit` implementation panics -/// (only when `debug_assertions` are enabled) if a span is exited on a span that -/// was entered on a different thread than where it was exited. Here, we specifically -/// test a future that is awaited across threads, as that is probably the most common -/// scenario where this can occur. +/// (only when `debug_assertions` are enabled) if a span, captured by Sentry, is exited +/// on a span that was entered on a different thread than where it was exited. +/// +/// Here, we specifically test a future that is awaited across threads, as that is +/// probably the most common scenario where this can occur. +/// +/// We use an info-level span, as that's the lowest level captured by Sentry by default. #[test] -fn future_cross_thread() { +fn future_cross_thread_info_span() { + const SPAN_NAME: &str = "future_cross_thread_info_span"; + let _guard = HubSwitchGuard::new(Hub::new_from_top(Hub::current()).into()); let transport = shared::init_sentry(1.0); - let mut future = Box::pin(span_across_await()); - - let (tx, rx) = mpsc::channel(); - - let thread1 = thread::spawn(move || { - let poll = future.as_mut().poll(&mut noop_context()); - assert!(poll.is_pending(), "future should be pending"); - tx.send(future).expect("failed to send future"); - }); - - let thread2 = thread::spawn(move || { - let _ = rx - .recv() - .expect("failed to receive future") - .as_mut() - .poll(&mut noop_context()); - }); + let span = tracing::info_span!(SPAN_NAME); - thread1 - .join() - .expect("thread 1 panicked, but should have completed"); + let thread2_result = futures_cross_thread_common(span); - let thread2_panic_message = thread2 - .join() + let thread2_panic_message = thread2_result .expect_err("thread2 did not panic, but it should have") .downcast::() .expect("expected thread2 to panic with a String message"); assert!( thread2_panic_message.starts_with("[SentryLayer] missing HubSwitchGuard on exit for span"), - "Thread 2's panicked, but not for the expected reason. It is also possible that the panic \ + "Thread 2 panicked, but not for the expected reason. It is also possible that the panic \ message was changed without updating this test." ); - // Despite the panic, the transaction should still get sent to Sentry - assert_transaction(transport.fetch_and_clear_envelopes()); + assert_transaction(transport.fetch_and_clear_envelopes(), SPAN_NAME); } -/// Counterpart to [`future_cross_thread`]; here, we check that the panic asserted in +/// Counterpart to [`future_cross_thread_info_span`]; here, we check that no panic occurs +/// if the span is not captured by Sentry. +/// +/// No panic should occur because we do not change the [`Hub`] for spans that we don't capture, +/// and so, we should not expect to pop a [`HubSwitchGuard`]. +#[test] +fn future_cross_thread_trace_span() { + let _guard = HubSwitchGuard::new(Hub::new_from_top(Hub::current()).into()); + let transport = shared::init_sentry(1.0); + + let span = tracing::trace_span!("future_cross_thread_trace_span"); + + futures_cross_thread_common(span) + .expect("no panic should occur for spans not captured by Sentry"); + + assert!( + transport.fetch_and_clear_envelopes().is_empty(), + "No envelopes should be sent for spans not captured by Sentry" + ); +} + +/// Counterpart to [`future_cross_thread_info_span`]; here, we check that the panic asserted in /// [`future_cross_thread`] is not triggered when the span is exited on the same thread /// that it was entered on. #[test] -fn futures_same_thread() { +fn futures_same_thread_info_span() { let _guard = HubSwitchGuard::new(Hub::new_from_top(Hub::current()).into()); let transport = shared::init_sentry(1.0); - let mut future = Box::pin(span_across_await()); + let span = tracing::info_span!("futures_same_thread_info_span"); + let mut future = Box::pin(span_across_await(span)); let thread = thread::spawn(move || { assert!( @@ -79,13 +87,48 @@ fn futures_same_thread() { }); thread.join().expect("thread should complete successfully"); - assert_transaction(transport.fetch_and_clear_envelopes()); + assert_transaction( + transport.fetch_and_clear_envelopes(), + "futures_same_thread_info_span", + ); +} + +/// Common logic for cross-thread tests. +/// +/// This function sets up the [`span_across_await`] future, then executes it such that +/// the span gets entered and exited from different threads. +fn futures_cross_thread_common(span: Span) -> Result<(), Box> { + let mut future = Box::pin(span_across_await(span)); + + let (tx, rx) = mpsc::channel(); + + let thread1 = thread::spawn(move || { + let poll = future.as_mut().poll(&mut noop_context()); + assert!(poll.is_pending(), "future should be pending"); + tx.send(future).expect("failed to send future"); + }); + + let thread2 = thread::spawn(move || { + let poll = rx + .recv() + .expect("failed to receive future") + .as_mut() + .poll(&mut noop_context()); + + assert!(poll.is_ready(), "future should be ready"); + }); + + thread1 + .join() + .expect("thread 1 panicked, but should have completed"); + + thread2.join() } /// Assert that the given envelopes contain exactly one [`Envelope`], /// containing exactly one [`EnvelopeItem`], which is a [`Transaction`], -/// with [`name`](Transaction::name) `"span_across_await"`. -fn assert_transaction(envelopes: Vec) { +/// with given [`name`](Transaction::name). +fn assert_transaction(envelopes: Vec, name: &str) { let envelope = get_and_assert_only_item(envelopes, "expected exactly one envelope"); let item = get_and_assert_only_item(envelope.into_items(), "expected exactly one item"); @@ -95,19 +138,17 @@ fn assert_transaction(envelopes: Vec) { EnvelopeItem::Transaction(Transaction { name: Some(expected_name), .. - }) if expected_name == "span_across_await" + }) if expected_name == name ), - "expected a Transaction item with name \"span_across_await\"" + "expected a Transaction item with name {name:?}" ); } -/// A helper function which creates and [`enter`s](tracing::Span::enter) -/// a [`Span`](tracing::Span), holding the returned +/// A helper function which and [`enter`s](tracing::Span::enter) +/// a given [`Span`](tracing::Span), holding the returned /// [`Entered<'_>`](tracing::span::Entered) guard across an `.await` boundary. -async fn span_across_await() { - let span = tracing::span!(Level::INFO, "span_across_await"); +async fn span_across_await(span: Span) { let _entered = span.enter(); - yield_once().await; // _entered dropped here, after .await call } From e85c7b399f03fee0f1152a90ca0a2e661e41d9ba Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Thu, 26 Feb 2026 17:06:31 +0100 Subject: [PATCH 05/10] fixup! Tests for `!Send` --- sentry-core/src/hub_impl.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/sentry-core/src/hub_impl.rs b/sentry-core/src/hub_impl.rs index 36994fa18..146caa871 100644 --- a/sentry-core/src/hub_impl.rs +++ b/sentry-core/src/hub_impl.rs @@ -27,6 +27,20 @@ thread_local! { pub struct SwitchGuard { inner: Option<(Arc, bool)>, /// Makes this type `!Send` while keeping it `Sync`. + /// + /// ```rust + /// # use sentry_core::HubSwitchGuard as SwitchGuard; + /// trait AssertSync: Sync {} + /// + /// impl AssertSync for SwitchGuard {} + /// ``` + /// + /// ```rust,compile_fail + /// # use sentry_core::HubSwitchGuard as SwitchGuard; + /// trait AssertSend: Send {} + /// + /// impl AssertSend for SwitchGuard {} + /// ``` _not_send: PhantomData>, } From d442adb39fcbc62e37d91c0ec01820da6a3949ab Mon Sep 17 00:00:00 2001 From: Daniel Szoke <7881302+szokeasaurusrex@users.noreply.github.com> Date: Fri, 27 Feb 2026 09:55:59 +0100 Subject: [PATCH 06/10] fixup! typo Co-authored-by: Lorenzo Cian <17258265+lcian@users.noreply.github.com> --- sentry-tracing/src/layer/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-tracing/src/layer/mod.rs b/sentry-tracing/src/layer/mod.rs index ed6dee496..4d5961c1d 100644 --- a/sentry-tracing/src/layer/mod.rs +++ b/sentry-tracing/src/layer/mod.rs @@ -343,7 +343,7 @@ where /// A tracing span can be entered and exited multiple times, for example, /// when using a `tracing::Instrumented` future. /// - /// Spans must be exited on the same span that they are entered. The + /// Spans must be exited on the same thread that they are entered. The /// `sentry-tracing` integration's behavior is undefined if spans are /// exited on threads other than the one they are entered from; /// specifically, doing so will likely cause data to bleed between From 2e5895afaa1d40b492ab98fb19ca78a3d85ee5f4 Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Fri, 27 Feb 2026 10:55:25 +0100 Subject: [PATCH 07/10] fixup! only assert panic with debug_assertions enabled --- .../tests/await_cross_thread_guard.rs | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/sentry-tracing/tests/await_cross_thread_guard.rs b/sentry-tracing/tests/await_cross_thread_guard.rs index 05807dd52..ad78b0c05 100644 --- a/sentry-tracing/tests/await_cross_thread_guard.rs +++ b/sentry-tracing/tests/await_cross_thread_guard.rs @@ -12,7 +12,7 @@ use tracing::Span; /// Test that the [`sentry_tracing::SentryLayer`]'s `on_exit` implementation panics /// (only when `debug_assertions` are enabled) if a span, captured by Sentry, is exited -/// on a span that was entered on a different thread than where it was exited. +/// on a different thread than where it was entered. /// /// Here, we specifically test a future that is awaited across threads, as that is /// probably the most common scenario where this can occur. @@ -29,16 +29,23 @@ fn future_cross_thread_info_span() { let thread2_result = futures_cross_thread_common(span); - let thread2_panic_message = thread2_result - .expect_err("thread2 did not panic, but it should have") - .downcast::() - .expect("expected thread2 to panic with a String message"); + // Panic should only occur when debug_assertions are enabled. + #[cfg(debug_assertions)] + { + let thread2_panic_message = thread2_result + .expect_err("thread2 did not panic, but it should have (debug_assertions enabled)") + .downcast::() + .expect("expected thread2 to panic with a String message"); - assert!( - thread2_panic_message.starts_with("[SentryLayer] missing HubSwitchGuard on exit for span"), - "Thread 2 panicked, but not for the expected reason. It is also possible that the panic \ - message was changed without updating this test." - ); + assert!( + thread2_panic_message.starts_with("[SentryLayer] missing HubSwitchGuard on exit for span"), + "Thread 2 panicked, but not for the expected reason. It is also possible that the panic \ + message was changed without updating this test." + ); + } + + #[cfg(not(debug_assertions))] + thread2_result.expect("thread2 should not panic if debug_assertions are disabled"); assert_transaction(transport.fetch_and_clear_envelopes(), SPAN_NAME); } From 8d4d76195a4fa5bd902bcdc28960f13e27e92262 Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Fri, 27 Feb 2026 10:57:58 +0100 Subject: [PATCH 08/10] fixup! changelog entry --- CHANGELOG.md | 3 +- docs/session-summary-2026-02-25.md | 51 ++++++++++++++++++++++++++++++ sentry-tracing/core | 0 3 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 docs/session-summary-2026-02-25.md create mode 100644 sentry-tracing/core diff --git a/CHANGELOG.md b/CHANGELOG.md index 53d046382..4f2ab9389 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,9 @@ ### Fixes -- Fixed thread corruption bug where `HubSwitchGuard` could be dropped on wrong thread ([#957](https://github.com/getsentry/sentry-rust/pull/957)) +- 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. +- We now fork the `Hub` every time a span is entered. This prevents data from leaking across spans ([#957](https://github.com/getsentry/sentry-rust/pull/957)). ## 0.46.2 diff --git a/docs/session-summary-2026-02-25.md b/docs/session-summary-2026-02-25.md new file mode 100644 index 000000000..11b3a51bd --- /dev/null +++ b/docs/session-summary-2026-02-25.md @@ -0,0 +1,51 @@ +# Session Summary (2026-02-25) + +## Goal +Refactor `sentry-tracing` invariant handling so the panic-or-log behavior is centralized in `sentry-core` macros. + +## Changes made + +### 1) Implemented `debug_panic_or_log!` +- **File:** `sentry-core/src/macros.rs` +- Replaced placeholder `todo!()` with real behavior: + - `panic!(...)` in debug builds (`cfg(debug_assertions)`) + - `sentry_debug!(...)` in non-debug builds +- Simplified implementation to forward macro args directly (removed intermediate `format_args!`). +- Added docstring: + - "Panics in debug builds and logs through `sentry_debug!` in non-debug builds." + +### 2) Added `debug_assert_or_log!` +- **File:** `sentry-core/src/macros.rs` +- New macro with two forms: + - `debug_assert_or_log!(cond)` + - `debug_assert_or_log!(cond, "...", args...)` +- Semantics: + - Evaluates condition once + - If condition is `false`, forwards to `debug_panic_or_log!` +- Default no-message form emits: + - `"assertion failed: {}", stringify!(cond)` + +### 3) Updated `sentry-tracing` `on_exit` +- **File:** `sentry-tracing/src/layer/mod.rs` +- Replaced inline panic/log block with: + - `sentry_core::debug_panic_or_log!(...)` +- Then updated again to use: + - `sentry_core::debug_assert_or_log!(popped.is_some(), ...)` + +### 4) Updated `sentry-tracing` `on_close` +- **File:** `sentry-tracing/src/layer/mod.rs` +- Replaced conditional debug log block: + - from `if removed_guards > 0 { sentry_debug!(...) }` + - to `sentry_core::debug_assert_or_log!(removed_guards == 0, ...)` + +## Validation +Ran checks after each stage: +- `cargo check -p sentry-tracing` +- `cargo check -p sentry-core -p sentry-tracing` + +All checks passed. + +## Outcome +Invariant handling is now centralized in reusable macros in `sentry-core`, with consistent behavior across `on_exit` and `on_close`: +- debug builds: panic +- non-debug builds: log via `sentry_debug!` diff --git a/sentry-tracing/core b/sentry-tracing/core new file mode 100644 index 000000000..e69de29bb From 875f2f952c3e8f1f67c53032fea3943b145bdcaa Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Fri, 27 Feb 2026 11:14:17 +0100 Subject: [PATCH 09/10] fixup! delete accidentally-committed files --- docs/session-summary-2026-02-25.md | 51 ------------------------------ sentry-tracing/core | 0 2 files changed, 51 deletions(-) delete mode 100644 docs/session-summary-2026-02-25.md delete mode 100644 sentry-tracing/core diff --git a/docs/session-summary-2026-02-25.md b/docs/session-summary-2026-02-25.md deleted file mode 100644 index 11b3a51bd..000000000 --- a/docs/session-summary-2026-02-25.md +++ /dev/null @@ -1,51 +0,0 @@ -# Session Summary (2026-02-25) - -## Goal -Refactor `sentry-tracing` invariant handling so the panic-or-log behavior is centralized in `sentry-core` macros. - -## Changes made - -### 1) Implemented `debug_panic_or_log!` -- **File:** `sentry-core/src/macros.rs` -- Replaced placeholder `todo!()` with real behavior: - - `panic!(...)` in debug builds (`cfg(debug_assertions)`) - - `sentry_debug!(...)` in non-debug builds -- Simplified implementation to forward macro args directly (removed intermediate `format_args!`). -- Added docstring: - - "Panics in debug builds and logs through `sentry_debug!` in non-debug builds." - -### 2) Added `debug_assert_or_log!` -- **File:** `sentry-core/src/macros.rs` -- New macro with two forms: - - `debug_assert_or_log!(cond)` - - `debug_assert_or_log!(cond, "...", args...)` -- Semantics: - - Evaluates condition once - - If condition is `false`, forwards to `debug_panic_or_log!` -- Default no-message form emits: - - `"assertion failed: {}", stringify!(cond)` - -### 3) Updated `sentry-tracing` `on_exit` -- **File:** `sentry-tracing/src/layer/mod.rs` -- Replaced inline panic/log block with: - - `sentry_core::debug_panic_or_log!(...)` -- Then updated again to use: - - `sentry_core::debug_assert_or_log!(popped.is_some(), ...)` - -### 4) Updated `sentry-tracing` `on_close` -- **File:** `sentry-tracing/src/layer/mod.rs` -- Replaced conditional debug log block: - - from `if removed_guards > 0 { sentry_debug!(...) }` - - to `sentry_core::debug_assert_or_log!(removed_guards == 0, ...)` - -## Validation -Ran checks after each stage: -- `cargo check -p sentry-tracing` -- `cargo check -p sentry-core -p sentry-tracing` - -All checks passed. - -## Outcome -Invariant handling is now centralized in reusable macros in `sentry-core`, with consistent behavior across `on_exit` and `on_close`: -- debug builds: panic -- non-debug builds: log via `sentry_debug!` diff --git a/sentry-tracing/core b/sentry-tracing/core deleted file mode 100644 index e69de29bb..000000000 From 0cf92a1a8354f848dc67852c3e613d952d3d9972 Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Fri, 27 Feb 2026 11:17:27 +0100 Subject: [PATCH 10/10] fixup! should_panic only with debug_assertions --- sentry-core/src/macros.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sentry-core/src/macros.rs b/sentry-core/src/macros.rs index 5dcdc78cd..647ce252e 100644 --- a/sentry-core/src/macros.rs +++ b/sentry-core/src/macros.rs @@ -113,14 +113,22 @@ mod tests { } #[test] + #[cfg(debug_assertions)] #[should_panic(expected = "assertion failed: 1 == 2")] fn debug_assert_or_log_panics_with_default_message_when_condition_fails() { crate::debug_assert_or_log!(1 == 2); } #[test] + #[cfg(debug_assertions)] #[should_panic(expected = "custom invariant message")] fn debug_assert_or_log_panics_with_custom_message_when_condition_fails() { crate::debug_assert_or_log!(false, "custom invariant message"); } + + #[test] + #[cfg(not(debug_assertions))] + fn no_panic_without_debug_assertions() { + crate::debug_assert_or_log!(false, "should not panic"); + } }