Skip to content

Commit 3d99c64

Browse files
committed
Merge #1014's head 54aae28 (the W3 tree takes the card from plain threads; node pipe) into m4b/spill2
2 parents c496b6d + 54aae28 commit 3d99c64

2 files changed

Lines changed: 716 additions & 151 deletions

File tree

‎prover/src/lfm/device_permit.rs‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,19 @@ pub fn hold_labeled(phase: &'static str) -> CardPermit {
442442
},
443443
};
444444
}
445+
// ⛔ Never from a rayon worker. A holder that waits inside rayon — a nested
446+
// `join` whose other half was stolen, a parallel iterator, another pool's
447+
// `install` — runs its pool's queued jobs on its own thread meanwhile, and
448+
// one of them that takes the card is a second hold on this thread (BIG 569's
449+
// arm N parked there; which run trips it is the scheduler's choice). Taken
450+
// from a plain thread, the card is held while rayon builds under it, and
451+
// that thread blocks instead of running anyone's jobs.
452+
#[cfg(feature = "parallel")]
453+
assert!(
454+
rayon::current_thread_index().is_none(),
455+
"the card permit is taken on a rayon worker ({phase}): a job it runs while it \
456+
waits inside rayon could take it again; take it from a plain thread"
457+
);
445458
assert!(
446459
!HELD_HERE.with(|h| h.get()),
447460
"the card permit is not reentrant and this thread already holds it; \
@@ -624,6 +637,82 @@ mod tests {
624637
disarm(&g);
625638
}
626639

640+
/// ★ THE RE-ENTRY BIG 569 HIT, FORCED — and now refused at its first hold.
641+
/// A holder on a rayon worker that waits on rayon while it holds the card —
642+
/// a nested `join` whose other half was stolen, a parallel iterator, another
643+
/// pool's `install` — runs queued jobs of its own pool on its own thread
644+
/// meanwhile, and one of them that takes the card is a second hold on one
645+
/// thread. The W3 tree built its leaves' artifacts as rayon jobs that each
646+
/// held the card around a build that uses rayon, so which run tripped it was
647+
/// down to the scheduler (568 clean, 569's arm N parked). Here `yield_now` is
648+
/// the wait and the sibling the only queued job: before the guard the
649+
/// sibling's hold re-entered every time (i-m4b's repro, m4b/permit-reentry);
650+
/// with it, the first hold on the worker is refused, before the card is
651+
/// taken, so nothing can re-enter.
652+
#[cfg(feature = "parallel")]
653+
#[test]
654+
fn armed_a_hold_on_a_rayon_worker_is_refused_before_a_sibling_can_take_it_again() {
655+
let g = ARM.lock().expect("the arm guard is never poisoned");
656+
disarm(&g);
657+
arm(2);
658+
let pool = rayon::ThreadPoolBuilder::new()
659+
.num_threads(1)
660+
.build()
661+
.expect("a one-thread pool");
662+
let ran = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
663+
pool.install(|| {
664+
rayon::join(
665+
|| {
666+
let _card = hold_labeled("build_artifacts");
667+
rayon::yield_now()
668+
},
669+
|| {
670+
let _card = hold_labeled("build_artifacts");
671+
},
672+
)
673+
})
674+
}));
675+
let payload = ran.expect_err("a hold on a rayon worker must be refused");
676+
let message = payload
677+
.downcast_ref::<String>()
678+
.cloned()
679+
.or_else(|| payload.downcast_ref::<&str>().map(|s| s.to_string()))
680+
.unwrap_or_default();
681+
assert!(
682+
message.contains("the card permit is taken on a rayon worker"),
683+
"another panic: {message}"
684+
);
685+
// Refused before the card was taken: it is free.
686+
drop(hold());
687+
disarm(&g);
688+
}
689+
690+
/// The other half: armed, a PLAIN thread holds the card while rayon works
691+
/// under it (a parallel build, as an artifact build does), and a second
692+
/// plain thread's hold waits for it rather than entering beside it.
693+
#[cfg(feature = "parallel")]
694+
#[test]
695+
fn armed_a_plain_thread_holds_the_card_while_rayon_builds_under_it() {
696+
use rayon::prelude::*;
697+
let g = ARM.lock().expect("the arm guard is never poisoned");
698+
disarm(&g);
699+
arm(2);
700+
let sum = std::thread::scope(|scope| {
701+
let card = hold_labeled("build_artifacts");
702+
let other = scope.spawn(|| {
703+
let _card = hold_labeled("build_artifacts");
704+
IN_FLIGHT.load(Ordering::SeqCst)
705+
});
706+
let sum: u64 = (0..10_000u64).into_par_iter().map(|x| x * x).sum();
707+
std::thread::sleep(Duration::from_millis(20));
708+
drop(card);
709+
assert_eq!(other.join().expect("the second holder"), 1);
710+
sum
711+
});
712+
assert_eq!(sum, (0..10_000u64).map(|x| x * x).sum::<u64>());
713+
disarm(&g);
714+
}
715+
627716
/// ★ A DEFERRED `multi_prove` WAITS FOR ITS LATCH, AND NOTHING ELSE DOES.
628717
///
629718
/// The worker's artifact hold goes straight to the card; its prove hold

0 commit comments

Comments
 (0)