fix(scheduler): entry-side check_cancelled in park_current — stop against a QUEUED actor was lossy
request_stop sets the flag and issues a wildcard unpark, but unpark no-ops on a Queued actor (the pending run "is" the wake). If the actor's first action on resume is a blocking park — no allocation, no check!() on the way — the wake-side-only check in park_current is unreachable: the actor parks with the stop flag already set and no wake is ever coming. The runtime then idles forever. Fix: check_cancelled at the ENTRY of park_current, before the switch. The remaining window (flag lands after the entry check, before the park) is covered by the existing protocol: the stop's unpark finds Running / the prep-to-park window, sets Notified, and the park-return re-queues into the wake-side check. Unwinding from the entry is the same unwind path as the wake side; leftover wait registrations are stale-epoch / dead-generation and self-clean at their wakers' failed CAS. This is bug #1 of the urus chunk-2 session (bug #2, the terminal wake, is the preceding commit). Repro: tests/cancel.rs stop_flagged_while_queued_lands_at_first_park — deadlocks without the fix (watchdog-bounded), passes in <50ms with it.
This commit is contained in:
@@ -212,6 +212,18 @@ pub fn yield_now() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub fn park_current() {
|
pub fn park_current() {
|
||||||
|
// Entry-side observation point: a stop flagged while we were QUEUED is
|
||||||
|
// otherwise lost — the stop's wildcard unpark no-ops on a Queued actor
|
||||||
|
// (the pending run "is" the wake), so if our first action on resume is
|
||||||
|
// this park, no wake is ever coming and the wake-side check below is
|
||||||
|
// unreachable. Checking here closes that hole; a flag that lands after
|
||||||
|
// this check is covered by the existing protocol (the stop's unpark
|
||||||
|
// finds Running / the prep-to-park window, sets Notified, and the
|
||||||
|
// park-return re-queues us into the wake-side check). Unwinding from
|
||||||
|
// here is the same unwind path as the wake side: leftover wait
|
||||||
|
// registrations are stale-epoch / dead-generation and self-clean at
|
||||||
|
// their wakers' failed CAS.
|
||||||
|
crate::preempt::check_cancelled();
|
||||||
runtime::set_yield_intent(YieldIntent::Park);
|
runtime::set_yield_intent(YieldIntent::Park);
|
||||||
unsafe { crate::context::switch_to_scheduler() };
|
unsafe { crate::context::switch_to_scheduler() };
|
||||||
// Observation point on the wakeup side of every blocking primitive
|
// Observation point on the wakeup side of every blocking primitive
|
||||||
|
|||||||
@@ -130,3 +130,61 @@ fn join_on_stopped_actor_returns_ok() {
|
|||||||
assert!(h.join().is_ok(), "join on a stopped actor returns Ok(())");
|
assert!(h.join().is_ok(), "join on a stopped actor returns Ok(())");
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Regression: `request_stop` against a QUEUED actor must not be lossy.
|
||||||
|
///
|
||||||
|
/// The stop flag is set, but the wildcard unpark no-ops on a Queued actor
|
||||||
|
/// (the pending run "is" the wake). If the actor's first action on resume
|
||||||
|
/// is a blocking park — no allocation, no `check!()` on the way — a
|
||||||
|
/// wake-side-only check in `park_current` never runs: the actor parks with
|
||||||
|
/// the stop flag already set, and nothing will ever wake it. The runtime
|
||||||
|
/// then idles forever (the root is parked on the monitor channel).
|
||||||
|
///
|
||||||
|
/// Fix: an entry-side `check_cancelled` in `park_current`. The remaining
|
||||||
|
/// window (flag set after the entry check, before the park lands) is closed
|
||||||
|
/// by the existing protocol: the stop's unpark then finds Running /
|
||||||
|
/// the prep-to-park window, sets Notified, and the park-return re-queues
|
||||||
|
/// into the wake-side check.
|
||||||
|
///
|
||||||
|
/// Watchdog harness: without the fix this deadlocks, so the runtime runs on
|
||||||
|
/// a side thread and the test fails on a timeout instead of hanging cargo.
|
||||||
|
#[test]
|
||||||
|
fn stop_flagged_while_queued_lands_at_first_park() {
|
||||||
|
use std::sync::mpsc;
|
||||||
|
use std::time::Duration;
|
||||||
|
|
||||||
|
let dropped = Arc::new(AtomicBool::new(false));
|
||||||
|
let saw_stopped = Arc::new(AtomicBool::new(false));
|
||||||
|
let (d, s) = (dropped.clone(), saw_stopped.clone());
|
||||||
|
|
||||||
|
let (done_tx, done_rx) = mpsc::channel::<()>();
|
||||||
|
std::thread::spawn(move || {
|
||||||
|
let rt = smarm::init(smarm::Config::exact(1));
|
||||||
|
rt.run(move || {
|
||||||
|
let h = spawn(move || {
|
||||||
|
let _g = DropFlag(d);
|
||||||
|
let (tx, rx) = channel::<u8>();
|
||||||
|
let _keep = tx; // keep the channel open: recv() parks
|
||||||
|
let _ = rx.recv(); // first observation point is this park
|
||||||
|
});
|
||||||
|
let pid = h.pid();
|
||||||
|
let down = monitor(pid);
|
||||||
|
// Stop while the child is still QUEUED — before it ever runs.
|
||||||
|
// The unpark no-ops; only the flag is left behind.
|
||||||
|
request_stop(pid);
|
||||||
|
let dn = down.rx.recv().expect("monitor channel closed before Down");
|
||||||
|
assert_eq!(dn.pid, pid);
|
||||||
|
if matches!(dn.reason, DownReason::Stopped) {
|
||||||
|
s.store(true, Ordering::SeqCst);
|
||||||
|
}
|
||||||
|
let _ = h.join();
|
||||||
|
});
|
||||||
|
let _ = done_tx.send(());
|
||||||
|
});
|
||||||
|
done_rx
|
||||||
|
.recv_timeout(Duration::from_secs(10))
|
||||||
|
.expect("runtime deadlocked: stop against a QUEUED actor was lost at its first park");
|
||||||
|
|
||||||
|
assert!(saw_stopped.load(Ordering::SeqCst), "expected DownReason::Stopped");
|
||||||
|
assert!(dropped.load(Ordering::SeqCst), "Drop guard must run during the cancellation unwind");
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user