Merge #1599

1599: Avoid lock poisoning by using parking_lot r=asomers a=rtzoeller

The synchronization primitives in the standard library are not panic-safe, however the `parking_lot` crate provides drop-in replacement primitives which are.

Add the `parking_lot` crate as a dev dependency and update the tests to use it. This should make it easier to narrow down which test(s) in a group of tests are actually failing, since the entire group will no longer fail due to poisoning.

Co-authored-by: Ryan Zoeller <[email protected]>

Author: bors[bot]

Cargo.toml

@@ -38,6 +38,7 @@ cc = "1"
 [dev-dependencies]
 assert-impl = "0.1"
 lazy_static = "1.2"
+parking_lot = "0.11.2"
 rand = "0.8"
 tempfile = "3.2.0"
 semver = "1.0.0"

test/sys/test_aio.rs

@@ -406,7 +406,7 @@ extern fn sigfunc(_: c_int) {
 #[test]
 #[cfg_attr(any(all(target_env = "musl", target_arch = "x86_64"), target_arch = "mips", target_arch = "mips64"), ignore)]
 fn test_write_sigev_signal() {
-    let _m = crate::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::SIGNAL_MTX.lock();
     let sa = SigAction::new(SigHandler::Handler(sigfunc),
                             SaFlags::SA_RESETHAND,
                             SigSet::empty());
@@ -544,7 +544,7 @@ fn test_liocb_listio_nowait() {
 #[cfg(not(any(target_os = "ios", target_os = "macos")))]
 #[cfg_attr(any(target_arch = "mips", target_arch = "mips64", target_env = "musl"), ignore)]
 fn test_liocb_listio_signal() {
-    let _m = crate::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::SIGNAL_MTX.lock();
     const INITIAL: &[u8] = b"abcdef123456";
     const WBUF: &[u8] = b"CDEF";
     let mut rbuf = vec![0; 4];

test/sys/test_ptrace.rs

@@ -69,7 +69,7 @@ fn test_ptrace_cont() {
 
     require_capability!("test_ptrace_cont", CAP_SYS_PTRACE);
 
-    let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::FORK_MTX.lock();
 
     // FIXME: qemu-user doesn't implement ptrace on all architectures
     // and retunrs ENOSYS in this case.
@@ -127,7 +127,7 @@ fn test_ptrace_interrupt() {
 
     require_capability!("test_ptrace_interrupt", CAP_SYS_PTRACE);
 
-    let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::FORK_MTX.lock();
 
     match unsafe{fork()}.expect("Error: Fork Failed") {
         Child => {
@@ -173,7 +173,7 @@ fn test_ptrace_syscall() {
 
     require_capability!("test_ptrace_syscall", CAP_SYS_PTRACE);
 
-    let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::FORK_MTX.lock();
 
     match unsafe{fork()}.expect("Error: Fork Failed") {
         Child => {

test/sys/test_select.rs

@@ -5,9 +5,7 @@ use nix::sys::time::{TimeSpec, TimeValLike};
 
 #[test]
 pub fn test_pselect() {
-    let _mtx = crate::SIGNAL_MTX
-        .lock()
-        .expect("Mutex got poisoned by another test");
+    let _mtx = crate::SIGNAL_MTX.lock();
 
     let (r1, w1) = pipe().unwrap();
     write(w1, b"hi!").unwrap();

test/sys/test_signal.rs

@@ -19,7 +19,7 @@ fn test_killpg_none() {
 
 #[test]
 fn test_old_sigaction_flags() {
-    let _m = crate::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::SIGNAL_MTX.lock();
 
     extern "C" fn handler(_: ::libc::c_int) {}
     let act = SigAction::new(
@@ -41,7 +41,7 @@ fn test_sigprocmask_noop() {
 
 #[test]
 fn test_sigprocmask() {
-    let _m = crate::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::SIGNAL_MTX.lock();
 
     // This needs to be a signal that rust doesn't use in the test harness.
     const SIGNAL: Signal = Signal::SIGCHLD;
@@ -89,15 +89,15 @@ extern fn test_sigaction_action(_: libc::c_int, _: *mut libc::siginfo_t, _: *mut
 #[test]
 #[cfg(not(target_os = "redox"))]
 fn test_signal_sigaction() {
-    let _m = crate::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::SIGNAL_MTX.lock();
 
     let action_handler = SigHandler::SigAction(test_sigaction_action);
     assert_eq!(unsafe { signal(Signal::SIGINT, action_handler) }.unwrap_err(), Errno::ENOTSUP);
 }
 
 #[test]
 fn test_signal() {
-    let _m = crate::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::SIGNAL_MTX.lock();
 
     unsafe { signal(Signal::SIGINT, SigHandler::SigIgn) }.unwrap();
     raise(Signal::SIGINT).unwrap();

test/sys/test_signalfd.rs

@@ -6,7 +6,7 @@ fn test_signalfd() {
     use nix::sys::signal::{self, raise, Signal, SigSet};
 
     // Grab the mutex for altering signals so we don't interfere with other tests.
-    let _m = crate::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::SIGNAL_MTX.lock();
 
     // Block the SIGUSR1 signal from automatic processing for this thread
     let mut mask = SigSet::empty();

test/sys/test_termios.rs

@@ -19,7 +19,7 @@ fn write_all(f: RawFd, buf: &[u8]) {
 #[test]
 fn test_tcgetattr_pty() {
     // openpty uses ptname(3) internally
-    let _m = crate::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::PTSNAME_MTX.lock();
 
     let pty = openpty(None, None).expect("openpty failed");
     assert!(termios::tcgetattr(pty.slave).is_ok());
@@ -46,7 +46,7 @@ fn test_tcgetattr_ebadf() {
 #[test]
 fn test_output_flags() {
     // openpty uses ptname(3) internally
-    let _m = crate::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::PTSNAME_MTX.lock();
 
     // Open one pty to get attributes for the second one
     let mut termios = {
@@ -88,7 +88,7 @@ fn test_output_flags() {
 #[test]
 fn test_local_flags() {
     // openpty uses ptname(3) internally
-    let _m = crate::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::PTSNAME_MTX.lock();
 
     // Open one pty to get attributes for the second one
     let mut termios = {

test/sys/test_uio.rs

@@ -214,7 +214,7 @@ fn test_process_vm_readv() {
     use crate::*;
 
     require_capability!("test_process_vm_readv", CAP_SYS_PTRACE);
-    let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::FORK_MTX.lock();
 
     // Pre-allocate memory in the child, since allocation isn't safe
     // post-fork (~= async-signal-safe)

test/sys/test_wait.rs

@@ -8,7 +8,7 @@ use libc::_exit;
 #[test]
 #[cfg(not(target_os = "redox"))]
 fn test_wait_signal() {
-    let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::FORK_MTX.lock();
 
     // Safe: The child only calls `pause` and/or `_exit`, which are async-signal-safe.
     match unsafe{fork()}.expect("Error: Fork Failed") {
@@ -25,7 +25,7 @@ fn test_wait_signal() {
 
 #[test]
 fn test_wait_exit() {
-    let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::FORK_MTX.lock();
 
     // Safe: Child only calls `_exit`, which is async-signal-safe.
     match unsafe{fork()}.expect("Error: Fork Failed") {
@@ -46,7 +46,7 @@ fn test_waitstatus_from_raw() {
 
 #[test]
 fn test_waitstatus_pid() {
-    let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = crate::FORK_MTX.lock();
 
     match unsafe{fork()}.unwrap() {
         Child => unsafe { _exit(0) },
@@ -97,7 +97,7 @@ mod ptrace {
     #[test]
     fn test_wait_ptrace() {
         require_capability!("test_wait_ptrace", CAP_SYS_PTRACE);
-        let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+        let _m = crate::FORK_MTX.lock();
 
         match unsafe{fork()}.expect("Error: Fork Failed") {
             Child => ptrace_child(),

test/test.rs

@@ -43,7 +43,7 @@ mod test_unistd;
 
 use std::os::unix::io::RawFd;
 use std::path::PathBuf;
-use std::sync::{Mutex, RwLock, RwLockWriteGuard};
+use parking_lot::{Mutex, RwLock, RwLockWriteGuard};
 use nix::unistd::{chdir, getcwd, read};
 
 
@@ -84,8 +84,7 @@ struct DirRestore<'a> {
 
 impl<'a> DirRestore<'a> {
     fn new() -> Self {
-        let guard = crate::CWD_LOCK.write()
-            .expect("Lock got poisoned by another test");
+        let guard = crate::CWD_LOCK.write();
         DirRestore{
             _g: guard,
             d: getcwd().unwrap(),