unwind from "block on" for panic/cancellation

We still record the same dependencies (or else the tests fail,
so +1 for test coverage).

This has the immediate advantage that we don't invoke the fallback
function twice for the repeated node in the cycle.

Also, fix a bug where revalidating cycles could lead to a
CycleParticipant error that is not caught (added a test for it).

src/derived.rs

@@ -174,7 +174,7 @@ where
         }
 
         db.salsa_runtime()
-            .report_query_read_and_panic_if_cycle_resulted(
+            .report_query_read_and_unwind_if_cycle_resulted(
                 slot.database_key_index(),
                 durability,
                 changed_at,

src/derived/slot.rs

@@ -3,7 +3,6 @@ use crate::derived::MemoizationPolicy;
 use crate::durability::Durability;
 use crate::lru::LruIndex;
 use crate::lru::LruNode;
-use crate::plumbing::{CycleDetected, CycleRecoveryStrategy};
 use crate::plumbing::{DatabaseOps, QueryFunction};
 use crate::revision::Revision;
 use crate::runtime::cycle_participant::CycleParticipant;
@@ -14,7 +13,7 @@ use crate::runtime::Runtime;
 use crate::runtime::RuntimeId;
 use crate::runtime::StampedValue;
 use crate::runtime::WaitResult;
-use crate::{Cancelled, Database, DatabaseKeyIndex, Event, EventKind, QueryDb};
+use crate::{Database, DatabaseKeyIndex, Event, EventKind, QueryDb};
 use log::{debug, info};
 use parking_lot::{RawRwLock, RwLock};
 use std::marker::PhantomData;
@@ -337,45 +336,10 @@ where
                 // not to gate future atomic reads.
                 anyone_waiting.store(true, Ordering::Relaxed);
 
-                match self.try_block_on_in_progress_query(db, runtime, other_id, state) {
-                    Ok(WaitResult::Panicked) => Cancelled::PropagatedPanic.throw(),
-                    Ok(WaitResult::Completed) => ProbeState::Retry,
-                    Err(CycleDetected {
-                        recovery_strategy,
-                        changed_at,
-                        durability,
-                        cycle,
-                    }) => match recovery_strategy {
-                        CycleRecoveryStrategy::Panic => cycle.throw(),
-
-                        // This is an interesting case. Here we have the 'final edge' of the
-                        // cycle:
-                        //
-                        //     C0 --> ... --> Cn --> C0
-                        //                        ^
-                        //                        :
-                        //         This edge -----+
-                        //
-                        // `self` reflects the query C0, but we are being executed from within
-                        // the current query of Cn. After having invoked `try_block_on_in_progress_query`,
-                        // we will have marked the active frames C0 .. Cn as cycle participants.
-                        // But we will need some value to return to Cn! Therefore, we invoke the cycle
-                        // fallback code here for C0 to create it.
-                        //
-                        // Cn will, in turn, record this dependency on C0 (along with the changed-at and
-                        // durability values that result) and then observe that it was a cycle participant.
-                        // After making that observation, Cn will panic and install its own recovery value.
-                        // This repeats until we have replaced the cached values of C0...Cn with their
-                        // recovery values.
-                        //
-                        // Note that the "cycle fallback" routine for C0 executes twice.
-                        CycleRecoveryStrategy::Fallback => ProbeState::UpToDate(StampedValue {
-                            value: Q::cycle_fallback(db, &cycle, &self.key),
-                            changed_at,
-                            durability,
-                        }),
-                    },
-                }
+                self.block_on_or_unwind(db, runtime, other_id, state);
+
+                // Other thread completely normally, so our value may be available now.
+                ProbeState::Retry
             }
 
             QueryState::Memoized(memo) => {
@@ -583,21 +547,15 @@ where
         }
     }
 
-    /// Helper:
-    ///
-    /// When we encounter an `InProgress` indicator, we need to either
-    /// report a cycle or else register ourselves to be notified when
-    /// that work completes. This helper does that. If a cycle is detected,
-    /// it returns immediately, but otherwise it blocks `self.database_key_index`
-    /// has completed and returns the corresponding result.
-    fn try_block_on_in_progress_query<MutexGuard>(
+    /// Helper: see [`Runtime::try_block_on_or_unwind`].
+    fn block_on_or_unwind<MutexGuard>(
         &self,
         db: &<Q as QueryDb<'_>>::DynDb,
         runtime: &Runtime,
         other_id: RuntimeId,
         mutex_guard: MutexGuard,
-    ) -> Result<WaitResult, CycleDetected> {
-        runtime.try_block_on(
+    ) {
+        runtime.block_on_or_unwind(
             db.ops_database(),
             self.database_key_index,
             other_id,

src/input.rs

@@ -110,7 +110,7 @@ where
         } = slot.stamped_value.read().clone();
 
         db.salsa_runtime()
-            .report_query_read_and_panic_if_cycle_resulted(
+            .report_query_read_and_unwind_if_cycle_resulted(
                 slot.database_key_index,
                 durability,
                 changed_at,

src/interned.rs

@@ -233,7 +233,7 @@ where
         let changed_at = slot.interned_at;
         let index = slot.index;
         db.salsa_runtime()
-            .report_query_read_and_panic_if_cycle_resulted(
+            .report_query_read_and_unwind_if_cycle_resulted(
                 slot.database_key_index,
                 INTERN_DURABILITY,
                 changed_at,
@@ -352,7 +352,7 @@ where
         let value = slot.value.clone();
         let interned_at = slot.interned_at;
         db.salsa_runtime()
-            .report_query_read_and_panic_if_cycle_resulted(
+            .report_query_read_and_unwind_if_cycle_resulted(
                 slot.database_key_index,
                 INTERN_DURABILITY,
                 interned_at,

src/plumbing.rs

@@ -19,22 +19,6 @@ pub use crate::interned::InternedStorage;
 pub use crate::interned::LookupInternedStorage;
 pub use crate::{revision::Revision, DatabaseKeyIndex, QueryDb, Runtime};
 
-#[derive(Clone, Debug)]
-pub struct CycleDetected {
-    /// Common recovery strategy to all participants in the cycle,
-    /// or [`CycleRecoveryStrategy::Panic`] otherwise.
-    pub(crate) recovery_strategy: CycleRecoveryStrategy,
-
-    /// Max `changed_at` from all participants in the cycle.
-    pub(crate) changed_at: Revision,
-
-    /// Min `durability` from all participants in the cycle.
-    pub(crate) durability: Durability,
-
-    /// Cycle participants.
-    pub(crate) cycle: Cycle,
-}
-
 /// Defines various associated types. An impl of this
 /// should be generated for your query-context type automatically by
 /// the `database_storage` macro, so you shouldn't need to mess