Skip to content

Commit a3ffa22

Browse files
authored
fix: Runaway for unchanged queries participating in cycle (#981)
* fix: Runaway for unchanged queries participating in cycle * Another regression test * Fix runaway situation * Discard changes to src/function/fetch.rs * Undo tracing changes * Move accumulated write outside of non-cycle branch * Short circuit if cycle head is executing * Inline * Update expected test output * Fix double execution * Simplify check in `validate_same_iteration` * Some more inline * Pass references
1 parent fb4cb24 commit a3ffa22

File tree

9 files changed

+433
-75
lines changed

9 files changed

+433
-75
lines changed

src/cycle.rs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ use thin_vec::{thin_vec, ThinVec};
5353

5454
use crate::key::DatabaseKeyIndex;
5555
use crate::sync::OnceLock;
56+
use crate::Revision;
5657

5758
/// The maximum number of times we'll fixpoint-iterate before panicking.
5859
///
@@ -237,16 +238,30 @@ pub(crate) fn empty_cycle_heads() -> &'static CycleHeads {
237238

238239
#[derive(Debug, PartialEq, Eq)]
239240
pub enum ProvisionalStatus {
240-
Provisional { iteration: IterationCount },
241-
Final { iteration: IterationCount },
241+
Provisional {
242+
iteration: IterationCount,
243+
verified_at: Revision,
244+
},
245+
Final {
246+
iteration: IterationCount,
247+
verified_at: Revision,
248+
},
242249
FallbackImmediate,
243250
}
244251

245252
impl ProvisionalStatus {
246253
pub(crate) const fn iteration(&self) -> Option<IterationCount> {
247254
match self {
248-
ProvisionalStatus::Provisional { iteration } => Some(*iteration),
249-
ProvisionalStatus::Final { iteration } => Some(*iteration),
255+
ProvisionalStatus::Provisional { iteration, .. } => Some(*iteration),
256+
ProvisionalStatus::Final { iteration, .. } => Some(*iteration),
257+
ProvisionalStatus::FallbackImmediate => None,
258+
}
259+
}
260+
261+
pub(crate) const fn verified_at(&self) -> Option<Revision> {
262+
match self {
263+
ProvisionalStatus::Provisional { verified_at, .. } => Some(*verified_at),
264+
ProvisionalStatus::Final { verified_at, .. } => Some(*verified_at),
250265
ProvisionalStatus::FallbackImmediate => None,
251266
}
252267
}

src/function.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,10 +345,16 @@ where
345345
if C::CYCLE_STRATEGY == CycleRecoveryStrategy::FallbackImmediate {
346346
ProvisionalStatus::FallbackImmediate
347347
} else {
348-
ProvisionalStatus::Final { iteration }
348+
ProvisionalStatus::Final {
349+
iteration,
350+
verified_at: memo.verified_at.load(),
351+
}
349352
}
350353
} else {
351-
ProvisionalStatus::Provisional { iteration }
354+
ProvisionalStatus::Provisional {
355+
iteration,
356+
verified_at: memo.verified_at.load(),
357+
}
352358
})
353359
}
354360

src/function/fetch.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use rustc_hash::FxHashMap;
2+
13
use crate::cycle::{CycleHeads, CycleRecoveryStrategy, IterationCount};
24
use crate::function::maybe_changed_after::VerifyCycleHeads;
35
use crate::function::memo::Memo;
@@ -172,7 +174,6 @@ where
172174
zalsa_local,
173175
database_key_index,
174176
old_memo,
175-
true,
176177
)
177178
{
178179
self.update_shallow(zalsa, database_key_index, old_memo, can_shallow_update);
@@ -182,17 +183,19 @@ where
182183
return unsafe { Some(self.extend_memo_lifetime(old_memo)) };
183184
}
184185

185-
let mut cycle_heads = VerifyCycleHeads::default();
186+
let mut cycle_heads = Vec::new();
187+
let mut participating_queries = FxHashMap::default();
188+
186189
let verify_result = self.deep_verify_memo(
187190
db,
188191
zalsa,
189192
old_memo,
190193
database_key_index,
191-
&mut cycle_heads,
194+
&mut VerifyCycleHeads::new(&mut cycle_heads, &mut participating_queries),
192195
can_shallow_update,
193196
);
194197

195-
if verify_result.is_unchanged() && !cycle_heads.has_any() {
198+
if verify_result.is_unchanged() && cycle_heads.is_empty() {
196199
// SAFETY: memo is present in memo_map and we have verified that it is
197200
// still valid for the current revision.
198201
return unsafe { Some(self.extend_memo_lifetime(old_memo)) };

0 commit comments

Comments
 (0)