Introduce RefUnwindSafe StorageHandle

Author: Veykril

src/storage.rs

@@ -1,3 +1,4 @@
+//! Public API facades for the implementation details of [`Zalsa`] and [`ZalsaLocal`].
 use std::{marker::PhantomData, panic::RefUnwindSafe, sync::Arc};
 
 use parking_lot::{Condvar, Mutex};
@@ -9,6 +10,50 @@ use crate::{
     Database, Event, EventKind,
 };
 
+/// A handle to non-local database state.
+pub struct StorageHandle<Db> {
+    // Note: Drop order is important, zalsa_impl needs to drop before coordinate
+    /// Reference to the database.
+    zalsa_impl: Arc<Zalsa>,
+
+    // Note: Drop order is important, coordinate needs to drop after zalsa_impl
+    /// Coordination data for cancellation of other handles when `zalsa_mut` is called.
+    /// This could be stored in Zalsa but it makes things marginally cleaner to keep it separate.
+    coordinate: CoordinateDrop,
+
+    /// We store references to `Db`
+    phantom: PhantomData<fn() -> Db>,
+}
+
+impl<Db: Database> Default for StorageHandle<Db> {
+    fn default() -> Self {
+        Self {
+            zalsa_impl: Arc::new(Zalsa::new::<Db>()),
+            coordinate: CoordinateDrop(Arc::new(Coordinate {
+                clones: Mutex::new(1),
+                cvar: Default::default(),
+            })),
+            phantom: PhantomData,
+        }
+    }
+}
+
+impl<Db> StorageHandle<Db> {
+    pub fn into_storage(self) -> Storage<Db> {
+        let StorageHandle {
+            zalsa_impl,
+            coordinate,
+            phantom,
+        } = self;
+        Storage {
+            zalsa_impl,
+            coordinate,
+            phantom,
+            zalsa_local: ZalsaLocal::new(),
+        }
+    }
+}
+
 /// Access the "storage" of a Salsa database: this is an internal plumbing trait
 /// automatically implemented by `#[salsa::db]` applied to a struct.
 ///
@@ -21,9 +66,8 @@ pub unsafe trait HasStorage: Database + Clone + Sized {
     fn storage_mut(&mut self) -> &mut Storage<Self>;
 }
 
-/// Concrete implementation of the [`Database`][] trait.
-/// Takes an optional type parameter `U` that allows you to thread your own data.
-pub struct Storage<Db: Database> {
+/// Concrete implementation of the [`Database`] trait with local state that can be used to drive computations.
+pub struct Storage<Db> {
     // Note: Drop order is important, zalsa_impl needs to drop before coordinate
     /// Reference to the database.
     zalsa_impl: Arc<Zalsa>,
@@ -39,13 +83,18 @@ pub struct Storage<Db: Database> {
     /// We store references to `Db`
     phantom: PhantomData<fn() -> Db>,
 }
+
 struct Coordinate {
     /// Counter of the number of clones of actor. Begins at 1.
     /// Incremented when cloned, decremented when dropped.
     clones: Mutex<usize>,
     cvar: Condvar,
 }
 
+// We cannot panic while holding a lock to `clones: Mutex<usize>` and therefore we cannot enter an
+// inconsistent state.
+impl RefUnwindSafe for Coordinate {}
+
 impl<Db: Database> Default for Storage<Db> {
     fn default() -> Self {
         Self {
@@ -61,6 +110,22 @@ impl<Db: Database> Default for Storage<Db> {
 }
 
 impl<Db: Database> Storage<Db> {
+    /// Discard the local state of this handle, turning it into a [`StorageHandle`] that is [`Sync`]
+    /// and [`std::panic::UnwindSafe`].
+    pub fn into_zalsa_handle(self) -> StorageHandle<Db> {
+        let Storage {
+            zalsa_impl,
+            coordinate,
+            phantom,
+            zalsa_local: _,
+        } = self;
+        StorageHandle {
+            zalsa_impl,
+            coordinate,
+            phantom,
+        }
+    }
+
     pub fn debug_input_entries<T>(&self) -> impl Iterator<Item = &input::Value<T>>
     where
         T: input::Configuration,
@@ -99,7 +164,9 @@ impl<Db: Database> Storage<Db> {
             .filter_map(|page| page.cast_type::<crate::table::Page<tracked_struct::Value<T>>>())
             .flat_map(|pages| pages.slots())
     }
+}
 
+impl<Db: Database> Storage<Db> {
     /// Access the `Arc<Zalsa>`. This should always be
     /// possible as `zalsa_impl` only becomes
     /// `None` once we are in the `Drop` impl.
@@ -150,8 +217,6 @@ unsafe impl<T: HasStorage> ZalsaDatabase for T {
     }
 }
 
-impl<Db: Database> RefUnwindSafe for Storage<Db> {}
-
 impl<Db: Database> Clone for Storage<Db> {
     fn clone(&self) -> Self {
         *self.coordinate.clones.lock() += 1;

src/zalsa.rs

@@ -3,6 +3,7 @@ use parking_lot::{Mutex, RwLock};
 use rustc_hash::FxHashMap;
 use std::any::{Any, TypeId};
 use std::marker::PhantomData;
+use std::panic::RefUnwindSafe;
 use std::thread::ThreadId;
 
 use crate::cycle::CycleRecoveryStrategy;
@@ -149,6 +150,13 @@ pub struct Zalsa {
     runtime: Runtime,
 }
 
+// Our fields locked behind Mutices and RwLocks cannot enter an inconsistent state due to panics
+// as they are all merely ID mappings with the exception of the `Runtime::dependency_graph`.
+// `Runtime::dependency_graph` does not invoke user queries though and as such will not arbitrarily
+// panic. The only way it may panic is by failing one of its asserts in which case we are already
+// in a broken state anyways.
+impl RefUnwindSafe for Zalsa {}
+
 impl Zalsa {
     pub(crate) fn new<Db: Database>() -> Self {
         Self {

tests/check_auto_traits.rs

@@ -0,0 +1,50 @@
+//! Test that auto trait impls exist as expected.
+
+use std::panic::UnwindSafe;
+
+use salsa::Database;
+use test_log::test;
+
+#[salsa::input]
+struct MyInput {
+    field: String,
+}
+
+#[salsa::tracked]
+struct MyTracked<'db> {
+    field: MyInterned<'db>,
+}
+
+#[salsa::interned]
+struct MyInterned<'db> {
+    field: String,
+}
+
+#[salsa::tracked]
+fn test(db: &dyn Database, input: MyInput) {
+    let input = is_send(is_sync(input));
+    let interned = is_send(is_sync(MyInterned::new(db, input.field(db).clone())));
+    let _tracked_struct = is_send(is_sync(MyTracked::new(db, interned)));
+}
+
+fn is_send<T: Send>(t: T) -> T {
+    t
+}
+
+fn is_sync<T: Send>(t: T) -> T {
+    t
+}
+
+fn is_unwind_safe<T: UnwindSafe>(t: T) -> T {
+    t
+}
+
+#[test]
+fn execute() {
+    let db = is_send(salsa::DatabaseImpl::new());
+    let _handle = is_send(is_sync(is_unwind_safe(
+        db.storage().clone().into_zalsa_handle(),
+    )));
+    let input = MyInput::new(&db, "Hello".to_string());
+    test(&db, input);
+}