Skip to content

fix!(core): Make HubSwitchGuard !Send to prevent thread corruption #957

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
szokeasaurusrex merged 10 commits into from
Feb 27, 2026
Merged

fix!(core): Make HubSwitchGuard !Send to prevent thread corruption #957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d0f44d8
fix!(core): Make HubSwitchGuard !Send to prevent thread corruption (#…
szokeasaurusrex Jan 14, 2026
ce0b7b3
fixup! logging for span exited on other thread
szokeasaurusrex Feb 26, 2026
9ef8d5f
fixup! Concurrency issue
szokeasaurusrex Feb 26, 2026
6c1fe0b
fixup! only expect pop for Sentry spans
szokeasaurusrex Feb 26, 2026
e85c7b3
fixup! Tests for `!Send`
szokeasaurusrex Feb 26, 2026
d442adb
fixup! typo
szokeasaurusrex Feb 27, 2026
2e5895a
fixup! only assert panic with debug_assertions enabled
szokeasaurusrex Feb 27, 2026
8d4d761
fixup! changelog entry
szokeasaurusrex Feb 27, 2026
875f2f9
fixup! delete accidentally-committed files
szokeasaurusrex Feb 27, 2026
0cf92a1
fixup! should_panic only with debug_assertions
szokeasaurusrex Feb 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@
- 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.
- 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

### New Features
Expand Down
30 changes: 26 additions & 4 deletions sentry-core/src/hub_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -19,10 +20,28 @@ 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<Hub>, 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<MutexGuard<'static, ()>>,
}

impl SwitchGuard {
Expand All @@ -41,7 +60,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<Arc<Hub>> {
Expand Down
59 changes: 59 additions & 0 deletions sentry-core/src/macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
() => {
Expand All @@ -73,3 +104,31 @@ 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]
#[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");
}
}
83 changes: 54 additions & 29 deletions sentry-tracing/src/layer.rs → sentry-tracing/src/layer/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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`]
Expand Down Expand Up @@ -234,9 +237,7 @@ fn record_fields<'a, K: AsRef<str> + Into<Cow<'a, str>>>(
/// the *current* span.
pub(super) struct SentrySpanData {
pub(super) sentry_span: TransactionOrSpan,
parent_sentry_span: Option<TransactionOrSpan>,
hub: Arc<sentry_core::Hub>,
hub_switch_guard: Option<sentry_core::HubSwitchGuard>,
}

impl<S> Layer<S> for SentryLayer<S>
Expand Down Expand Up @@ -334,47 +335,66 @@ 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
/// 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 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
/// [`Hub`]s in unexpected ways.
fn on_enter(&self, id: &span::Id, ctx: Context<'_, S>) {
let span = match ctx.span(id) {
Some(span) => span,
None => return,
};

let mut extensions = span.extensions_mut();
if let Some(data) = extensions.get_mut::<SentrySpanData>() {
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::<SentrySpanData>() {
// 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::<SentrySpanData>() {
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, 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()
|| ctx
.span(id)
.is_none_or(|span| span.extensions().get::<SentrySpanData>().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."
);
}

/// 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>) {
Expand Down Expand Up @@ -503,6 +523,11 @@ fn extract_span_data(

thread_local! {
static VISITOR_BUFFER: RefCell<String> = const { RefCell::new(String::new()) };
/// Hub switch guards keyed by span ID.
///
/// Guard bookkeeping is thread-local by design. Correctness expects
/// balanced enter/exit callbacks on the same thread.
static SPAN_GUARDS: RefCell<SpanGuardStack> = RefCell::new(SpanGuardStack::new());
}

/// Records all span fields into a `BTreeMap`, reusing a mutable `String` as buffer.
Expand Down
79 changes: 79 additions & 0 deletions sentry-tracing/src/layer/span_guard_stack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
//! 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<SpanId, Vec<HubSwitchGuard>>,
}

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.
#[must_use]
pub(super) fn pop(&mut self, id: SpanId) -> Option<HubSwitchGuard> {
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,
}
}
}

#[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());
}
}
Loading