From 5d3799fcb820a14472f9b530a4839f68e587876e Mon Sep 17 00:00:00 2001 From: ClawSeven Date: Thu, 28 Dec 2023 15:15:11 +0800 Subject: [PATCH] [libos] Fix deadlock in signal implementions --- src/libos/src/interrupt/mod.rs | 21 ++++++--------------- src/libos/src/signal/do_sigpending.rs | 3 ++- src/libos/src/signal/do_sigreturn.rs | 12 ++++++------ src/libos/src/signal/do_sigtimedwait.rs | 4 ++-- 4 files changed, 16 insertions(+), 24 deletions(-) diff --git a/src/libos/src/interrupt/mod.rs b/src/libos/src/interrupt/mod.rs index df47ed97..6d4f5e7c 100644 --- a/src/libos/src/interrupt/mod.rs +++ b/src/libos/src/interrupt/mod.rs @@ -42,21 +42,12 @@ pub fn broadcast_interrupts() -> Result { return true; } - let sig_mask_guard = thread.sig_mask().read().unwrap(); - let interested = !*sig_mask_guard; - drop(sig_mask_guard); - - let thread_pending_sig = thread.sig_queues().read().unwrap().pending() & interested; - if !thread_pending_sig.empty() { - return true; - } - - let process_pending_sig = - thread.process().sig_queues().read().unwrap().pending() & interested; - if !process_pending_sig.empty() { - return true; - } - false + let interested = !*thread.sig_mask().read().unwrap(); + // In the nightly-2022-10-22 Rust compiler, this expression holds two nested read locks. + // However, in the stable-2023-12-21 Rust compiler, the expression drops the temporary variables + // (including: read lock guard) after each division code completes. + !((thread.process().sig_queues().read().unwrap().pending() & interested).empty()) + || !((thread.sig_queues().read().unwrap().pending() & interested).empty()) }; let num_signaled_threads = crate::process::table::get_all_threads() diff --git a/src/libos/src/signal/do_sigpending.rs b/src/libos/src/signal/do_sigpending.rs index a1050ac6..66312762 100644 --- a/src/libos/src/signal/do_sigpending.rs +++ b/src/libos/src/signal/do_sigpending.rs @@ -6,9 +6,10 @@ pub fn do_sigpending() -> Result { let thread = current!(); let process = thread.process(); + let blocked = *thread.sig_mask().read().unwrap(); let pending = (thread.sig_queues().read().unwrap().pending() | process.sig_queues().read().unwrap().pending()) - & *thread.sig_mask().read().unwrap(); + & blocked; Ok(pending) } diff --git a/src/libos/src/signal/do_sigreturn.rs b/src/libos/src/signal/do_sigreturn.rs index abc36963..b53cad5b 100644 --- a/src/libos/src/signal/do_sigreturn.rs +++ b/src/libos/src/signal/do_sigreturn.rs @@ -125,12 +125,12 @@ fn do_deliver_signal(thread: &ThreadRef, process: &ProcessRef, cpu_context: &mut let sig_mask = *thread.sig_mask().read().unwrap() | *thread.sig_tmp_mask().read().unwrap(); - let signal_opt = process - .sig_queues() - .write() - .unwrap() - .dequeue(&sig_mask) - .or_else(|| thread.sig_queues().write().unwrap().dequeue(&sig_mask)); + // Don't use `Option` or_else to avoid nested write locks + let mut signal_opt = process.sig_queues().write().unwrap().dequeue(&sig_mask); + if signal_opt.is_none() { + signal_opt = thread.sig_queues().write().unwrap().dequeue(&sig_mask); + } + if signal_opt.is_none() { return; } diff --git a/src/libos/src/signal/do_sigtimedwait.rs b/src/libos/src/signal/do_sigtimedwait.rs index 28c6f97f..b7a0f76e 100644 --- a/src/libos/src/signal/do_sigtimedwait.rs +++ b/src/libos/src/signal/do_sigtimedwait.rs @@ -163,8 +163,8 @@ impl Drop for PendingSigWaiter { } fn has_interest_signal(interest: &SigSet, thread: &ThreadRef, process: &ProcessRef) -> bool { - let pending = (thread.sig_queues().read().unwrap().pending() - | process.sig_queues().read().unwrap().pending()) + let pending = (process.sig_queues().read().unwrap().pending() + | thread.sig_queues().read().unwrap().pending()) & *interest; !pending.empty()