Add test for panic in nested cycle

Author: MichaReiser

src/function/sync.rs

@@ -243,6 +243,10 @@ impl<'me> ClaimGuard<'me> {
     fn release_panicking(&self) {
         let mut syncs = self.sync_table.syncs.lock();
         let state = syncs.remove(&self.key_index).expect("key claimed twice?");
+        tracing::debug!(
+            "Release claim on {:?} due to panic",
+            self.database_key_index()
+        );
 
         self.release(state, WaitResult::Panicked);
     }

tests/parallel/cycle_nested_deep_panic.rs

@@ -0,0 +1,104 @@
+// Shuttle doesn't like panics inside of its runtime.
+#![cfg(not(feature = "shuttle"))]
+
+//! Tests that salsa doesn't get stuck after a panic in a nested cycle function.
+
+use crate::sync::thread;
+use crate::{Knobs, KnobsDatabase};
+use std::panic::catch_unwind;
+
+use salsa::CycleRecoveryAction;
+
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy, salsa::Update)]
+struct CycleValue(u32);
+
+const MIN: CycleValue = CycleValue(0);
+const MAX: CycleValue = CycleValue(3);
+
+#[salsa::tracked(cycle_fn=cycle_fn, cycle_initial=initial)]
+fn query_a(db: &dyn KnobsDatabase) -> CycleValue {
+    db.signal(1);
+    query_b(db)
+}
+
+#[salsa::tracked(cycle_fn=cycle_fn, cycle_initial=initial)]
+fn query_b(db: &dyn KnobsDatabase) -> CycleValue {
+    let c_value = query_c(db);
+    CycleValue(c_value.0 + 1).min(MAX)
+}
+
+#[salsa::tracked(cycle_fn=cycle_fn, cycle_initial=initial)]
+fn query_c(db: &dyn KnobsDatabase) -> CycleValue {
+    let d_value = query_d(db);
+
+    if d_value > CycleValue(0) {
+        let _e_value = query_e(db);
+        let _b = query_b(db);
+        db.wait_for(2);
+        db.signal(3);
+        panic!("Dragons are real");
+    } else {
+        let a_value = query_a(db);
+        CycleValue(d_value.0.max(a_value.0))
+    }
+}
+
+#[salsa::tracked(cycle_fn=cycle_fn, cycle_initial=initial)]
+fn query_d(db: &dyn KnobsDatabase) -> CycleValue {
+    query_c(db)
+}
+
+#[salsa::tracked(cycle_fn=cycle_fn, cycle_initial=initial)]
+fn query_e(db: &dyn KnobsDatabase) -> CycleValue {
+    query_c(db)
+}
+
+fn cycle_fn(
+    _db: &dyn KnobsDatabase,
+    _value: &CycleValue,
+    _count: u32,
+) -> CycleRecoveryAction<CycleValue> {
+    CycleRecoveryAction::Iterate
+}
+
+fn initial(_db: &dyn KnobsDatabase) -> CycleValue {
+    MIN
+}
+
+#[test_log::test]
+fn the_test() {
+    tracing::debug!("Starting new run");
+    let db_t1 = Knobs::default();
+    let db_t2 = db_t1.clone();
+    let db_t3 = db_t1.clone();
+    let db_t4 = db_t1.clone();
+
+    let t1 = thread::spawn(move || {
+        let _span = tracing::debug_span!("t1", thread_id = ?thread::current().id()).entered();
+
+        let result = query_a(&db_t1);
+        result
+    });
+    let t2 = thread::spawn(move || {
+        let _span = tracing::debug_span!("t4", thread_id = ?thread::current().id()).entered();
+        db_t4.wait_for(1);
+        db_t4.signal(2);
+        query_b(&db_t4)
+    });
+    let t3 = thread::spawn(move || {
+        let _span = tracing::debug_span!("t2", thread_id = ?thread::current().id()).entered();
+        db_t2.wait_for(1);
+        query_d(&db_t2)
+    });
+
+    let r_t1 = t1.join();
+    let r_t2 = t2.join();
+    let r_t3 = t3.join();
+
+    assert!(r_t1.is_err());
+    assert!(r_t2.is_err());
+    assert!(r_t3.is_err());
+
+    // Pulling the cycle again at a later point should still result in a panic.
+    assert!(catch_unwind(|| query_d(&db_t3)).is_err());
+}

tests/parallel/main.rs

@@ -9,6 +9,7 @@ mod cycle_ab_peeping_c;
 mod cycle_nested_deep;
 mod cycle_nested_deep_conditional;
 mod cycle_nested_deep_conditional_changed;
+mod cycle_nested_deep_panic;
 mod cycle_nested_three_threads;
 mod cycle_nested_three_threads_changed;
 mod cycle_panic;