Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixes

- Fixed a bug that could cause the SDK to panic and, in some cases, panic while handling the panic, aborting the process ([#1241](https://github.com/getsentry/sentry-rust/pull/1241)).

## 0.48.4

### New Features
Expand Down
19 changes: 13 additions & 6 deletions sentry-core/src/hub_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,19 @@ impl SwitchGuard {
}

fn swap(&mut self) -> Option<Arc<Hub>> {
self.inner.take().map(|mut hub| {
THREAD_HUB.with(|thread_hub| {
let mut thread_hub = thread_hub.borrow_mut();
std::mem::swap(&mut *thread_hub, &mut hub);
hub
})
self.inner.take().and_then(|mut hub| {
// We use `try_with` to access the `THREAD_HUB`, intentionally ignoring any errors that
// result. If `try_with` errors, this is because the `THREAD_HUB` local key has been
// destroyed, which means that there is nothing to swap with, making this operation
// pointless. Further, the destruction of the thread-local indicates that the thread
// is likely shutting down.
THREAD_HUB
.try_with(|thread_hub| {
let mut thread_hub = thread_hub.borrow_mut();
std::mem::swap(&mut *thread_hub, &mut hub);
hub
})
.ok()
})
}
}
Expand Down
68 changes: 68 additions & 0 deletions sentry-core/tests/switch_guard_tls_teardown.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
//! Regression tests for [issue #1237].
//!
//! The tests cover both possible destruction orders for the thread-local
//! storage containing `HubSwitchGuard` values and `THREAD_HUB`. TLS destruction
//! order is not guaranteed, so testing both orders increases the likelihood
//! that a reintroduced bug is detected on a given platform.
//!
//! [issue #1237]: https://github.com/getsentry/sentry-rust/issues/1237

use std::cell::RefCell;
use std::sync::Arc;
use std::thread;

use sentry::{Hub, HubSwitchGuard};

/// Initializes the guard TLS before `THREAD_HUB`, reliably reproducing the
/// reported failure on macOS targets. The test passes once the bug is fixed.
#[test]
fn switch_guard_tolerates_destroyed_thread_hub() {
thread_local! {
static STORED_GUARD: RefCell<Option<HubSwitchGuard>> =
const { RefCell::new(None) };
}

thread::spawn(|| {
// Initialize this TLS value before THREAD_HUB.
STORED_GUARD.with(|_| {});

// Hub::current initializes THREAD_HUB.
let current = Hub::current();
let replacement = Arc::new(Hub::new_from_top(current));
let guard = HubSwitchGuard::new(replacement);

// Keep the guard alive until TLS teardown.
STORED_GUARD.with_borrow_mut(|slot| {
*slot = Some(guard);
});
})
.join()
.expect("worker should exit without panicking");
}

/// Initializes the guard TLS after `THREAD_HUB` and covers the other possible
/// TLS destruction order. TLS destruction order is not guaranteed, so this test
/// helps detect a reintroduced bug on platforms where the first order does not
/// reliably occur.
#[test]
fn switch_guard_drops_before_thread_hub() {
thread_local! {
static STORED_GUARD: RefCell<Option<HubSwitchGuard>> =
const { RefCell::new(None) };
}

thread::spawn(|| {
// Hub::current initializes THREAD_HUB before STORED_GUARD.
let current = Hub::current();
let replacement = Arc::new(Hub::new_from_top(current));
let guard = HubSwitchGuard::new(replacement);

// Keep the guard alive until TLS teardown, but initialize its TLS
// after THREAD_HUB.
STORED_GUARD.with_borrow_mut(|slot| {
*slot = Some(guard);
});
})
.join()
.expect("worker should exit without panicking");
}
Loading