Skip to content

Commit 72fe6c7

Browse files
committed
Disable local cancellation while within a cycle computation
1 parent 9d8760e commit 72fe6c7

File tree

5 files changed

+44
-20
lines changed

5 files changed

+44
-20
lines changed

src/function/execute.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -109,13 +109,18 @@ where
109109

110110
(new_value, completed_query)
111111
}
112-
CycleRecoveryStrategy::Fixpoint => self.execute_maybe_iterate(
113-
db,
114-
opt_old_memo,
115-
&mut claim_guard,
116-
zalsa_local,
117-
memo_ingredient_index,
118-
),
112+
CycleRecoveryStrategy::Fixpoint => {
113+
let was_disabled = zalsa_local.set_cancellation_disabled(true);
114+
let res = self.execute_maybe_iterate(
115+
db,
116+
opt_old_memo,
117+
&mut claim_guard,
118+
zalsa_local,
119+
memo_ingredient_index,
120+
);
121+
zalsa_local.set_cancellation_disabled(was_disabled);
122+
res
123+
}
119124
};
120125

121126
if let Some(old_memo) = opt_old_memo {

src/function/memo.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,11 @@ impl<'db, C: Configuration> Memo<'db, C> {
181181
TryClaimHeadsResult::Running(running) => {
182182
all_cycles = false;
183183
if !running.block_on(zalsa) {
184-
// FIXME: Handle cancellation properly?
184+
// We cannot handle local cancellations in fixpoints
185+
// so we treat it as a general cancellation / panic.
186+
//
187+
// We shouldn't hit this though as we disable local cancellation
188+
// in cycles.
185189
crate::Cancelled::PropagatedPanic.throw();
186190
}
187191
}

src/function/sync.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ pub enum SyncOwner {
288288

289289
/// The query's lock ownership has been transferred to another query.
290290
/// E.g. if `a` transfers its ownership to `b`, then only the thread in the critical path
291-
/// to complete b` can claim `a` (in most instances, only the thread owning `b` can claim `a`).
291+
/// to complete `b` can claim `a` (in most instances, only the thread owning `b` can claim `a`).
292292
///
293293
/// The thread owning `a` is stored in the `DependencyGraph`.
294294
///
@@ -469,7 +469,7 @@ impl<'me> ClaimGuard<'me> {
469469
impl Drop for ClaimGuard<'_> {
470470
fn drop(&mut self) {
471471
if thread::panicking() {
472-
if self.zalsa_local.is_cancelled() {
472+
if self.zalsa_local.should_trigger_local_cancellation() {
473473
self.release_cancelled();
474474
} else {
475475
self.release_panicking();

src/zalsa.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ impl Zalsa {
301301
#[inline]
302302
pub(crate) fn unwind_if_revision_cancelled(&self, zalsa_local: &ZalsaLocal) {
303303
self.event(&|| crate::Event::new(crate::EventKind::WillCheckCancellation));
304-
if zalsa_local.is_cancelled() {
304+
if zalsa_local.should_trigger_local_cancellation() {
305305
zalsa_local.unwind_cancelled();
306306
}
307307
if self.runtime().load_cancellation_flag() {

src/zalsa_local.rs

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::fmt;
33
use std::fmt::Formatter;
44
use std::panic::UnwindSafe;
55
use std::ptr::{self, NonNull};
6-
use std::sync::atomic::Ordering;
6+
use std::sync::atomic::{AtomicU8, Ordering};
77
use std::sync::Arc;
88

99
use rustc_hash::FxHashMap;
@@ -47,21 +47,32 @@ pub struct ZalsaLocal {
4747

4848
/// A cancellation token that can be used to cancel a query computation for a specific local `Database`.
4949
#[derive(Default, Clone, Debug)]
50-
pub struct CancellationToken(Arc<AtomicBool>);
50+
pub struct CancellationToken(Arc<AtomicU8>);
5151

5252
impl CancellationToken {
53+
const CANCELLED_MASK: u8 = 0b01;
54+
const DISABLED_MASK: u8 = 0b10;
55+
5356
/// Inform the database to cancel the current query computation.
5457
pub fn cancel(&self) {
55-
self.0.store(true, Ordering::Relaxed);
58+
self.0.fetch_or(Self::CANCELLED_MASK, Ordering::Relaxed);
5659
}
5760

5861
/// Check if the query computation has been requested to be cancelled.
5962
pub fn is_cancelled(&self) -> bool {
60-
self.0.load(Ordering::Relaxed)
63+
self.0.load(Ordering::Relaxed) & Self::CANCELLED_MASK != 0
6164
}
6265

63-
pub(crate) fn uncancel(&self) {
64-
self.0.store(false, Ordering::Relaxed);
66+
fn set_cancellation_disabled(&self, disabled: bool) -> bool {
67+
self.0.fetch_or((disabled as u8) << 1, Ordering::Relaxed) & Self::DISABLED_MASK != 0
68+
}
69+
70+
fn should_trigger_local_cancellation(&self) -> bool {
71+
self.0.load(Ordering::Relaxed) == Self::CANCELLED_MASK
72+
}
73+
74+
fn reset(&self) {
75+
self.0.store(0, Ordering::Relaxed);
6576
}
6677
}
6778

@@ -433,12 +444,12 @@ impl ZalsaLocal {
433444

434445
#[inline]
435446
pub(crate) fn uncancel(&self) {
436-
self.cancelled.uncancel();
447+
self.cancelled.reset();
437448
}
438449

439450
#[inline]
440-
pub fn is_cancelled(&self) -> bool {
441-
self.cancelled.0.load(Ordering::Relaxed)
451+
pub fn should_trigger_local_cancellation(&self) -> bool {
452+
self.cancelled.should_trigger_local_cancellation()
442453
}
443454

444455
#[cold]
@@ -450,6 +461,10 @@ impl ZalsaLocal {
450461
pub(crate) fn unwind_cancelled(&self) {
451462
Cancelled::Local.throw();
452463
}
464+
465+
pub(crate) fn set_cancellation_disabled(&self, was_disabled: bool) -> bool {
466+
self.cancelled.set_cancellation_disabled(was_disabled)
467+
}
453468
}
454469

455470
// Okay to implement as `ZalsaLocal`` is !Sync

0 commit comments

Comments
 (0)