diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a4ebc6b..28b09f2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ **Internal**: -- Simplify `Hub::run` and `SentryFuture` by using a scope-guard and `arc-swap` for `Hub` switching. ([#524](https://github.com/getsentry/sentry-rust/pull/524)) +- Simplify `Hub::run` and `SentryFuture` by using a scope-guard for `Hub` switching. ([#524](https://github.com/getsentry/sentry-rust/pull/524), [#529](https://github.com/getsentry/sentry-rust/pull/529)) **Thank you**: diff --git a/sentry-core/Cargo.toml b/sentry-core/Cargo.toml index 26ca759b..6726b4b0 100644 --- a/sentry-core/Cargo.toml +++ b/sentry-core/Cargo.toml @@ -21,7 +21,7 @@ harness = false [features] default = [] -client = ["rand", "arc-swap"] +client = ["rand"] # I would love to just have a `log` feature, but this is used inside a macro, # and macros actually expand features (and extern crate) where they are used! debug-logs = ["dep:log"] @@ -42,7 +42,6 @@ build_id = { version = "0.2.1", optional = true } findshlibs = { version = "=0.10.2", optional = true } rustc_version_runtime = { version = "0.2.1", optional = true } indexmap = { version = "1.9.1", optional = true } -arc-swap = { version = "1.5.1", optional = true } [target.'cfg(target_family = "unix")'.dependencies] pprof = { version = "0.11.0", optional = true, default-features = false } diff --git a/sentry-core/src/hub_impl.rs b/sentry-core/src/hub_impl.rs index c3e5db11..76a91c3d 100644 --- a/sentry-core/src/hub_impl.rs +++ b/sentry-core/src/hub_impl.rs @@ -1,11 +1,10 @@ -use std::cell::Cell; +use std::cell::{Cell, UnsafeCell}; use std::sync::{Arc, PoisonError, RwLock}; use std::thread; use crate::Scope; use crate::{scope::Stack, Client, Hub}; -use arc_swap::ArcSwap; use once_cell::sync::Lazy; static PROCESS_HUB: Lazy<(Arc, thread::ThreadId)> = Lazy::new(|| { @@ -16,8 +15,8 @@ static PROCESS_HUB: Lazy<(Arc, thread::ThreadId)> = Lazy::new(|| { }); thread_local! { - static THREAD_HUB: (ArcSwap, Cell) = ( - ArcSwap::from_pointee(Hub::new_from_top(&PROCESS_HUB.0)), + static THREAD_HUB: (UnsafeCell>, Cell) = ( + UnsafeCell::new(Arc::new(Hub::new_from_top(&PROCESS_HUB.0))), Cell::new(PROCESS_HUB.1 == thread::current().id()) ); } @@ -27,25 +26,26 @@ pub(crate) struct SwitchGuard { } impl SwitchGuard { - pub(crate) fn new(hub: Arc) -> Self { - let inner = THREAD_HUB.with(|(old_hub, is_process_hub)| { - { - let old_hub = old_hub.load(); - if std::ptr::eq(old_hub.as_ref(), hub.as_ref()) { - return None; - } + pub(crate) fn new(mut hub: Arc) -> Self { + let inner = THREAD_HUB.with(|(thread_hub, is_process_hub)| { + // SAFETY: `thread_hub` will always be a valid thread local hub, + // by definition not shared between threads. + let thread_hub = unsafe { &mut *thread_hub.get() }; + if std::ptr::eq(thread_hub.as_ref(), hub.as_ref()) { + return None; } - let old_hub = old_hub.swap(hub); + std::mem::swap(thread_hub, &mut hub); let was_process_hub = is_process_hub.replace(false); - Some((old_hub, was_process_hub)) + Some((hub, was_process_hub)) }); SwitchGuard { inner } } fn swap(&mut self) -> Option> { - if let Some((old_hub, was_process_hub)) = self.inner.take() { - Some(THREAD_HUB.with(|(hub, is_process_hub)| { - let hub = hub.swap(old_hub); + if let Some((mut hub, was_process_hub)) = self.inner.take() { + Some(THREAD_HUB.with(|(thread_hub, is_process_hub)| { + let thread_hub = unsafe { &mut *thread_hub.get() }; + std::mem::swap(thread_hub, &mut hub); if was_process_hub { is_process_hub.set(true); } @@ -150,7 +150,7 @@ impl Hub { if is_process_hub.get() { f(&PROCESS_HUB.0) } else { - f(&hub.load()) + f(unsafe { &*hub.get() }) } }) }