Skip to content

Commit 21b407b

Browse files
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<MutexGuard<'static, ()>> 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 Fixes #946 Refs RUST-130 Refs RUST-132 Co-Authored-By: Claude Sonnet 4.5 (1M context) <[email protected]>
1 parent 02e446e commit 21b407b

File tree

2 files changed

+53
-15
lines changed

2 files changed

+53
-15
lines changed

sentry-core/src/hub_impl.rs

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::cell::{Cell, UnsafeCell};
2-
use std::sync::{Arc, LazyLock, PoisonError, RwLock};
2+
use std::marker::PhantomData;
3+
use std::sync::{Arc, LazyLock, MutexGuard, PoisonError, RwLock};
34
use std::thread;
45

56
use crate::Scope;
@@ -19,10 +20,14 @@ thread_local! {
1920
);
2021
}
2122

22-
/// A Hub switch guard used to temporarily swap
23-
/// active hub in thread local storage.
23+
/// A guard that temporarily swaps the active hub in thread-local storage.
24+
///
25+
/// This type is `!Send` because it manages thread-local state and must be
26+
/// dropped on the same thread where it was created.
2427
pub struct SwitchGuard {
2528
inner: Option<(Arc<Hub>, bool)>,
29+
/// Makes this type `!Send` while keeping it `Sync`.
30+
_not_send: PhantomData<MutexGuard<'static, ()>>,
2631
}
2732

2833
impl SwitchGuard {
@@ -41,7 +46,10 @@ impl SwitchGuard {
4146
let was_process_hub = is_process_hub.replace(false);
4247
Some((hub, was_process_hub))
4348
});
44-
SwitchGuard { inner }
49+
SwitchGuard {
50+
inner,
51+
_not_send: PhantomData,
52+
}
4553
}
4654

4755
fn swap(&mut self) -> Option<Arc<Hub>> {
@@ -192,3 +200,22 @@ impl Hub {
192200
.with_mut(|stack| f(Arc::make_mut(&mut stack.top_mut().scope)))
193201
}
194202
}
203+
204+
#[cfg(all(test, feature = "test"))]
205+
mod tests {
206+
use super::*;
207+
208+
#[test]
209+
fn test_switch_guard_restores_hub_on_same_thread() {
210+
let original_hub = Hub::current();
211+
let custom_hub = Arc::new(Hub::new(None, Arc::new(Default::default())));
212+
assert!(!Arc::ptr_eq(&original_hub, &custom_hub));
213+
214+
{
215+
let _guard = SwitchGuard::new(custom_hub.clone());
216+
assert!(Arc::ptr_eq(&Hub::current(), &custom_hub));
217+
}
218+
219+
assert!(Arc::ptr_eq(&Hub::current(), &original_hub));
220+
}
221+
}

sentry-tracing/src/layer.rs

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::borrow::Cow;
22
use std::cell::RefCell;
3-
use std::collections::BTreeMap;
3+
use std::collections::{BTreeMap, HashMap};
44
use std::sync::Arc;
55

66
use bitflags::bitflags;
@@ -236,7 +236,6 @@ pub(super) struct SentrySpanData {
236236
pub(super) sentry_span: TransactionOrSpan,
237237
parent_sentry_span: Option<TransactionOrSpan>,
238238
hub: Arc<sentry_core::Hub>,
239-
hub_switch_guard: Option<sentry_core::HubSwitchGuard>,
240239
}
241240

242241
impl<S> Layer<S> for SentryLayer<S>
@@ -338,24 +337,26 @@ where
338337
sentry_span,
339338
parent_sentry_span,
340339
hub,
341-
hub_switch_guard: None,
342340
});
343341
}
344342

345343
/// Sets entered span as *current* sentry span. A tracing span can be
346-
/// entered and existed multiple times, for example, when using a `tracing::Instrumented` future.
344+
/// entered and exited multiple times, for example, when using a `tracing::Instrumented` future.
347345
fn on_enter(&self, id: &span::Id, ctx: Context<'_, S>) {
348346
let span = match ctx.span(id) {
349347
Some(span) => span,
350348
None => return,
351349
};
352350

353-
let mut extensions = span.extensions_mut();
354-
if let Some(data) = extensions.get_mut::<SentrySpanData>() {
355-
data.hub_switch_guard = Some(sentry_core::HubSwitchGuard::new(data.hub.clone()));
351+
let extensions = span.extensions();
352+
if let Some(data) = extensions.get::<SentrySpanData>() {
353+
let guard = sentry_core::HubSwitchGuard::new(data.hub.clone());
354+
SPAN_GUARDS.with(|guards| {
355+
guards.borrow_mut().insert(id.clone(), guard);
356+
});
356357
data.hub.configure_scope(|scope| {
357358
scope.set_span(Some(data.sentry_span.clone()));
358-
})
359+
});
359360
}
360361
}
361362

@@ -366,18 +367,24 @@ where
366367
None => return,
367368
};
368369

369-
let mut extensions = span.extensions_mut();
370-
if let Some(data) = extensions.get_mut::<SentrySpanData>() {
370+
let extensions = span.extensions();
371+
if let Some(data) = extensions.get::<SentrySpanData>() {
371372
data.hub.configure_scope(|scope| {
372373
scope.set_span(data.parent_sentry_span.clone());
373374
});
374-
data.hub_switch_guard.take();
375375
}
376+
SPAN_GUARDS.with(|guards| {
377+
guards.borrow_mut().remove(id);
378+
});
376379
}
377380

378381
/// When a span gets closed, finish the underlying sentry span, and set back
379382
/// its parent as the *current* sentry span.
380383
fn on_close(&self, id: span::Id, ctx: Context<'_, S>) {
384+
SPAN_GUARDS.with(|guards| {
385+
guards.borrow_mut().remove(&id);
386+
});
387+
381388
let span = match ctx.span(&id) {
382389
Some(span) => span,
383390
None => return,
@@ -503,6 +510,10 @@ fn extract_span_data(
503510

504511
thread_local! {
505512
static VISITOR_BUFFER: RefCell<String> = const { RefCell::new(String::new()) };
513+
/// Hub switch guards keyed by span ID. Stored in thread-local so guards are
514+
/// always dropped on the same thread where they were created.
515+
static SPAN_GUARDS: RefCell<HashMap<span::Id, sentry_core::HubSwitchGuard>> =
516+
RefCell::new(HashMap::new());
506517
}
507518

508519
/// Records all span fields into a `BTreeMap`, reusing a mutable `String` as buffer.

0 commit comments

Comments
 (0)