ref(hub): Add ProcessHub struct

sentry-core/src/hub_impl.rs

@@ -1,22 +1,21 @@
-use std::cell::{Cell, UnsafeCell};
+use std::cell::{Cell, RefCell};
 use std::marker::PhantomData;
 use std::sync::{Arc, LazyLock, MutexGuard, PoisonError, RwLock};
 use std::thread;
+use std::thread::ThreadId;
 
 use crate::Scope;
 use crate::{scope::Stack, Client, Hub};
 
-static PROCESS_HUB: LazyLock<(Arc<Hub>, thread::ThreadId)> = LazyLock::new(|| {
-    (
-        Arc::new(Hub::new(None, Arc::new(Default::default()))),
-        thread::current().id(),
-    )
+static PROCESS_HUB: LazyLock<ProcessHub> = LazyLock::new(|| ProcessHub {
+    hub: Arc::new(Hub::new(None, Arc::new(Default::default()))),
+    thread: thread::current().id(),
 });
 
 thread_local! {
-    static THREAD_HUB: (UnsafeCell<Arc<Hub>>, Cell<bool>) = (
-        UnsafeCell::new(Arc::new(Hub::new_from_top(&PROCESS_HUB.0))),
-        Cell::new(PROCESS_HUB.1 == thread::current().id())
+    static THREAD_HUB: (RefCell<Arc<Hub>>, Cell<bool>) = (
+        RefCell::new(Arc::new(Hub::new_from_top(&PROCESS_HUB.hub))),
+        Cell::new(PROCESS_HUB.thread == thread::current().id())
     );
 }
 
@@ -50,13 +49,11 @@ impl SwitchGuard {
     /// to the previous one.
     pub fn new(mut hub: Arc<Hub>) -> 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() };
+            let mut thread_hub = thread_hub.borrow_mut();
             if std::ptr::eq(thread_hub.as_ref(), hub.as_ref()) {
                 return None;
             }
-            std::mem::swap(thread_hub, &mut hub);
+            std::mem::swap(&mut *thread_hub, &mut hub);
             let was_process_hub = is_process_hub.replace(false);
             Some((hub, was_process_hub))
         });
@@ -69,8 +66,8 @@ impl SwitchGuard {
     fn swap(&mut self) -> Option<Arc<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);
+                let mut thread_hub = thread_hub.borrow_mut();
+                std::mem::swap(&mut *thread_hub, &mut hub);
                 if was_process_hub {
                     is_process_hub.set(true);
                 }
@@ -156,7 +153,7 @@ impl Hub {
     /// This is similar to [`Hub::current`] but instead of picking the
     /// current thread's hub it returns the main thread's hub instead.
     pub fn main() -> Arc<Hub> {
-        PROCESS_HUB.0.clone()
+        PROCESS_HUB.hub.clone()
     }
 
     /// Invokes the callback with the default hub.
@@ -169,9 +166,13 @@ impl Hub {
     {
         THREAD_HUB.with(|(hub, is_process_hub)| {
             if is_process_hub.get() {
-                f(&PROCESS_HUB.0)
+                f(&PROCESS_HUB.hub)
             } else {
-                f(unsafe { &*hub.get() })
+                // Bind `hub` as `hub.borrow().clone()`.
+                // It is essential we drop `hub.borrow()` before the callback, otherwise we will
+                // panic if the callback calls `Hub::run`, so we need the binding.
+                let hub = hub.borrow().clone();
+                f(&hub)
             }
         })
     }
@@ -214,3 +215,29 @@ impl Hub {
             .with_mut(|stack| f(Arc::make_mut(&mut stack.top_mut().scope)))
     }
 }
+
+/// Helper struct for storing the [`PROCESS_HUB`].
+struct ProcessHub {
+    /// The process's main hub.
+    hub: Arc<Hub>,
+    /// The thread on which the main hub was initialized.
+    thread: ThreadId,
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Regression test for [`Hub::with`], ensuring that the `RefCell` borrow is not held during the callback.
+    ///
+    /// If we hold the `RefCell` borrow during the callback, this would panic.
+    #[test]
+    fn hub_run_inside_with_scope() {
+        let outer_hub = Arc::new(Hub::new(None, Default::default()));
+        let inner_hub = Arc::new(Hub::new(None, Default::default()));
+
+        Hub::run(outer_hub, || {
+            Hub::with(|_| Hub::run(inner_hub, || {}));
+        });
+    }
+}