Skip to content

Avoid lock poisoning by using parking_lot #1599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions test/sys/test_aio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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];
Expand Down
6 changes: 3 additions & 3 deletions test/sys/test_ptrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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 => {
Expand Down
4 changes: 1 addition & 3 deletions test/sys/test_select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
8 changes: 4 additions & 4 deletions test/sys/test_signal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion test/sys/test_signalfd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions test/sys/test_termios.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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 = {
Expand Down Expand Up @@ -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 = {
Expand Down
2 changes: 1 addition & 1 deletion test/sys/test_uio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions test/sys/test_wait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand All @@ -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") {
Expand All @@ -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) },
Expand Down Expand Up @@ -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(),
Expand Down
5 changes: 2 additions & 3 deletions test/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};


Expand Down Expand Up @@ -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(),
Expand Down
32 changes: 15 additions & 17 deletions test/test_kmod/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ use tempfile::{tempdir, TempDir};
use crate::*;

fn compile_kernel_module() -> (PathBuf, String, TempDir) {
let _m = crate::FORK_MTX
.lock()
.expect("Mutex got poisoned by another test");
let _m = crate::FORK_MTX.lock();

let tmp_dir = tempdir().expect("unable to create temporary build directory");

Expand Down Expand Up @@ -41,8 +39,8 @@ use std::io::Read;
#[test]
fn test_finit_and_delete_module() {
require_capability!("test_finit_and_delete_module", CAP_SYS_MODULE);
let _m0 = crate::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
let _m1 = crate::CWD_LOCK.read().expect("Mutex got poisoned by another test");
let _m0 = crate::KMOD_MTX.lock();
let _m1 = crate::CWD_LOCK.read();

let (kmod_path, kmod_name, _kmod_dir) = compile_kernel_module();

Expand All @@ -59,8 +57,8 @@ fn test_finit_and_delete_module() {
#[test]
fn test_finit_and_delete_module_with_params() {
require_capability!("test_finit_and_delete_module_with_params", CAP_SYS_MODULE);
let _m0 = crate::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
let _m1 = crate::CWD_LOCK.read().expect("Mutex got poisoned by another test");
let _m0 = crate::KMOD_MTX.lock();
let _m1 = crate::CWD_LOCK.read();

let (kmod_path, kmod_name, _kmod_dir) = compile_kernel_module();

Expand All @@ -80,8 +78,8 @@ fn test_finit_and_delete_module_with_params() {
#[test]
fn test_init_and_delete_module() {
require_capability!("test_init_and_delete_module", CAP_SYS_MODULE);
let _m0 = crate::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
let _m1 = crate::CWD_LOCK.read().expect("Mutex got poisoned by another test");
let _m0 = crate::KMOD_MTX.lock();
let _m1 = crate::CWD_LOCK.read();

let (kmod_path, kmod_name, _kmod_dir) = compile_kernel_module();

Expand All @@ -100,8 +98,8 @@ fn test_init_and_delete_module() {
#[test]
fn test_init_and_delete_module_with_params() {
require_capability!("test_init_and_delete_module_with_params", CAP_SYS_MODULE);
let _m0 = crate::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
let _m1 = crate::CWD_LOCK.read().expect("Mutex got poisoned by another test");
let _m0 = crate::KMOD_MTX.lock();
let _m1 = crate::CWD_LOCK.read();

let (kmod_path, kmod_name, _kmod_dir) = compile_kernel_module();

Expand All @@ -121,8 +119,8 @@ fn test_init_and_delete_module_with_params() {
#[test]
fn test_finit_module_invalid() {
require_capability!("test_finit_module_invalid", CAP_SYS_MODULE);
let _m0 = crate::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
let _m1 = crate::CWD_LOCK.read().expect("Mutex got poisoned by another test");
let _m0 = crate::KMOD_MTX.lock();
let _m1 = crate::CWD_LOCK.read();

let kmod_path = "/dev/zero";

Expand All @@ -135,8 +133,8 @@ fn test_finit_module_invalid() {
#[test]
fn test_finit_module_twice_and_delete_module() {
require_capability!("test_finit_module_twice_and_delete_module", CAP_SYS_MODULE);
let _m0 = crate::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
let _m1 = crate::CWD_LOCK.read().expect("Mutex got poisoned by another test");
let _m0 = crate::KMOD_MTX.lock();
let _m1 = crate::CWD_LOCK.read();

let (kmod_path, kmod_name, _kmod_dir) = compile_kernel_module();

Expand All @@ -157,8 +155,8 @@ fn test_finit_module_twice_and_delete_module() {
#[test]
fn test_delete_module_not_loaded() {
require_capability!("test_delete_module_not_loaded", CAP_SYS_MODULE);
let _m0 = crate::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
let _m1 = crate::CWD_LOCK.read().expect("Mutex got poisoned by another test");
let _m0 = crate::KMOD_MTX.lock();
let _m1 = crate::CWD_LOCK.read();

let result = delete_module(&CString::new("hello").unwrap(), DeleteModuleFlags::empty());

Expand Down
18 changes: 9 additions & 9 deletions test/test_pty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ fn test_explicit_close() {
#[test]
#[cfg(any(target_os = "android", target_os = "linux"))]
fn test_ptsname_equivalence() {
let _m = crate::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
let _m = crate::PTSNAME_MTX.lock();

// Open a new PTTY master
let master_fd = posix_openpt(OFlag::O_RDWR).unwrap();
Expand All @@ -46,7 +46,7 @@ fn test_ptsname_equivalence() {
#[test]
#[cfg(any(target_os = "android", target_os = "linux"))]
fn test_ptsname_copy() {
let _m = crate::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
let _m = crate::PTSNAME_MTX.lock();

// Open a new PTTY master
let master_fd = posix_openpt(OFlag::O_RDWR).unwrap();
Expand Down Expand Up @@ -80,7 +80,7 @@ fn test_ptsname_r_copy() {
#[test]
#[cfg(any(target_os = "android", target_os = "linux"))]
fn test_ptsname_unique() {
let _m = crate::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
let _m = crate::PTSNAME_MTX.lock();

// Open a new PTTY master
let master1_fd = posix_openpt(OFlag::O_RDWR).unwrap();
Expand All @@ -98,7 +98,7 @@ fn test_ptsname_unique() {

/// Common setup for testing PTTY pairs
fn open_ptty_pair() -> (PtyMaster, File) {
let _m = crate::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
let _m = crate::PTSNAME_MTX.lock();

// Open a new PTTY master
let master = posix_openpt(OFlag::O_RDWR).expect("posix_openpt failed");
Expand All @@ -114,7 +114,7 @@ fn open_ptty_pair() -> (PtyMaster, File) {
let slave_fd = open(Path::new(&slave_name), OFlag::O_RDWR, stat::Mode::empty()).unwrap();

#[cfg(target_os = "illumos")]
// TODO: rewrite using ioctl!
// TODO: rewrite using ioctl!
#[allow(clippy::comparison_chain)]
{
use libc::{ioctl, I_FIND, I_PUSH};
Expand Down Expand Up @@ -187,7 +187,7 @@ fn test_write_ptty_pair() {
#[test]
fn test_openpty() {
// 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).unwrap();
assert!(pty.master > 0);
Expand Down Expand Up @@ -222,7 +222,7 @@ fn test_openpty() {
#[test]
fn test_openpty_with_termios() {
// 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 = {
Expand Down Expand Up @@ -273,9 +273,9 @@ fn test_forkpty() {
use nix::sys::signal::*;
use nix::sys::wait::wait;
// forkpty calls openpty which uses ptname(3) internally.
let _m0 = crate::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
let _m0 = crate::PTSNAME_MTX.lock();
// forkpty spawns a child process
let _m1 = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
let _m1 = crate::FORK_MTX.lock();

let string = "naninani\n";
let echoed_string = "naninani\r\n";
Expand Down
Loading