Skip to content

Commit 09bddc3

Browse files
committed
Avoid lock poisoning by using parking_lot
parking_lot provides synchronization primitives which aren't poisoned on panic. This makes it easier to determine which tests are failing, as a test failure no longer causes all subsequent tests using that mutex to fail.
1 parent b91bce3 commit 09bddc3

File tree

13 files changed

+59
-63
lines changed

13 files changed

+59
-63
lines changed

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ cc = "1"
3838
[dev-dependencies]
3939
assert-impl = "0.1"
4040
lazy_static = "1.2"
41+
parking_lot = "0.11.2"
4142
rand = "0.8"
4243
tempfile = "3.2.0"
4344
semver = "1.0.0"

test/sys/test_aio.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ extern fn sigfunc(_: c_int) {
406406
#[test]
407407
#[cfg_attr(any(all(target_env = "musl", target_arch = "x86_64"), target_arch = "mips", target_arch = "mips64"), ignore)]
408408
fn test_write_sigev_signal() {
409-
let _m = crate::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
409+
let _m = crate::SIGNAL_MTX.lock();
410410
let sa = SigAction::new(SigHandler::Handler(sigfunc),
411411
SaFlags::SA_RESETHAND,
412412
SigSet::empty());
@@ -544,7 +544,7 @@ fn test_liocb_listio_nowait() {
544544
#[cfg(not(any(target_os = "ios", target_os = "macos")))]
545545
#[cfg_attr(any(target_arch = "mips", target_arch = "mips64", target_env = "musl"), ignore)]
546546
fn test_liocb_listio_signal() {
547-
let _m = crate::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
547+
let _m = crate::SIGNAL_MTX.lock();
548548
const INITIAL: &[u8] = b"abcdef123456";
549549
const WBUF: &[u8] = b"CDEF";
550550
let mut rbuf = vec![0; 4];

test/sys/test_ptrace.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ fn test_ptrace_cont() {
6969

7070
require_capability!("test_ptrace_cont", CAP_SYS_PTRACE);
7171

72-
let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
72+
let _m = crate::FORK_MTX.lock();
7373

7474
// FIXME: qemu-user doesn't implement ptrace on all architectures
7575
// and retunrs ENOSYS in this case.
@@ -127,7 +127,7 @@ fn test_ptrace_interrupt() {
127127

128128
require_capability!("test_ptrace_interrupt", CAP_SYS_PTRACE);
129129

130-
let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
130+
let _m = crate::FORK_MTX.lock();
131131

132132
match unsafe{fork()}.expect("Error: Fork Failed") {
133133
Child => {
@@ -173,7 +173,7 @@ fn test_ptrace_syscall() {
173173

174174
require_capability!("test_ptrace_syscall", CAP_SYS_PTRACE);
175175

176-
let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
176+
let _m = crate::FORK_MTX.lock();
177177

178178
match unsafe{fork()}.expect("Error: Fork Failed") {
179179
Child => {

test/sys/test_select.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@ use nix::sys::time::{TimeSpec, TimeValLike};
55

66
#[test]
77
pub fn test_pselect() {
8-
let _mtx = crate::SIGNAL_MTX
9-
.lock()
10-
.expect("Mutex got poisoned by another test");
8+
let _mtx = crate::SIGNAL_MTX.lock();
119

1210
let (r1, w1) = pipe().unwrap();
1311
write(w1, b"hi!").unwrap();

test/sys/test_signal.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ fn test_killpg_none() {
1919

2020
#[test]
2121
fn test_old_sigaction_flags() {
22-
let _m = crate::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
22+
let _m = crate::SIGNAL_MTX.lock();
2323

2424
extern "C" fn handler(_: ::libc::c_int) {}
2525
let act = SigAction::new(
@@ -41,7 +41,7 @@ fn test_sigprocmask_noop() {
4141

4242
#[test]
4343
fn test_sigprocmask() {
44-
let _m = crate::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
44+
let _m = crate::SIGNAL_MTX.lock();
4545

4646
// This needs to be a signal that rust doesn't use in the test harness.
4747
const SIGNAL: Signal = Signal::SIGCHLD;
@@ -89,15 +89,15 @@ extern fn test_sigaction_action(_: libc::c_int, _: *mut libc::siginfo_t, _: *mut
8989
#[test]
9090
#[cfg(not(target_os = "redox"))]
9191
fn test_signal_sigaction() {
92-
let _m = crate::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
92+
let _m = crate::SIGNAL_MTX.lock();
9393

9494
let action_handler = SigHandler::SigAction(test_sigaction_action);
9595
assert_eq!(unsafe { signal(Signal::SIGINT, action_handler) }.unwrap_err(), Errno::ENOTSUP);
9696
}
9797

9898
#[test]
9999
fn test_signal() {
100-
let _m = crate::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
100+
let _m = crate::SIGNAL_MTX.lock();
101101

102102
unsafe { signal(Signal::SIGINT, SigHandler::SigIgn) }.unwrap();
103103
raise(Signal::SIGINT).unwrap();

test/sys/test_signalfd.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ fn test_signalfd() {
66
use nix::sys::signal::{self, raise, Signal, SigSet};
77

88
// Grab the mutex for altering signals so we don't interfere with other tests.
9-
let _m = crate::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
9+
let _m = crate::SIGNAL_MTX.lock();
1010

1111
// Block the SIGUSR1 signal from automatic processing for this thread
1212
let mut mask = SigSet::empty();

test/sys/test_termios.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ fn write_all(f: RawFd, buf: &[u8]) {
1919
#[test]
2020
fn test_tcgetattr_pty() {
2121
// openpty uses ptname(3) internally
22-
let _m = crate::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
22+
let _m = crate::PTSNAME_MTX.lock();
2323

2424
let pty = openpty(None, None).expect("openpty failed");
2525
assert!(termios::tcgetattr(pty.slave).is_ok());
@@ -46,7 +46,7 @@ fn test_tcgetattr_ebadf() {
4646
#[test]
4747
fn test_output_flags() {
4848
// openpty uses ptname(3) internally
49-
let _m = crate::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
49+
let _m = crate::PTSNAME_MTX.lock();
5050

5151
// Open one pty to get attributes for the second one
5252
let mut termios = {
@@ -88,7 +88,7 @@ fn test_output_flags() {
8888
#[test]
8989
fn test_local_flags() {
9090
// openpty uses ptname(3) internally
91-
let _m = crate::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
91+
let _m = crate::PTSNAME_MTX.lock();
9292

9393
// Open one pty to get attributes for the second one
9494
let mut termios = {

test/sys/test_uio.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ fn test_process_vm_readv() {
214214
use crate::*;
215215

216216
require_capability!("test_process_vm_readv", CAP_SYS_PTRACE);
217-
let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
217+
let _m = crate::FORK_MTX.lock();
218218

219219
// Pre-allocate memory in the child, since allocation isn't safe
220220
// post-fork (~= async-signal-safe)

test/sys/test_wait.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use libc::_exit;
88
#[test]
99
#[cfg(not(target_os = "redox"))]
1010
fn test_wait_signal() {
11-
let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
11+
let _m = crate::FORK_MTX.lock();
1212

1313
// Safe: The child only calls `pause` and/or `_exit`, which are async-signal-safe.
1414
match unsafe{fork()}.expect("Error: Fork Failed") {
@@ -25,7 +25,7 @@ fn test_wait_signal() {
2525

2626
#[test]
2727
fn test_wait_exit() {
28-
let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
28+
let _m = crate::FORK_MTX.lock();
2929

3030
// Safe: Child only calls `_exit`, which is async-signal-safe.
3131
match unsafe{fork()}.expect("Error: Fork Failed") {
@@ -46,7 +46,7 @@ fn test_waitstatus_from_raw() {
4646

4747
#[test]
4848
fn test_waitstatus_pid() {
49-
let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
49+
let _m = crate::FORK_MTX.lock();
5050

5151
match unsafe{fork()}.unwrap() {
5252
Child => unsafe { _exit(0) },
@@ -97,7 +97,7 @@ mod ptrace {
9797
#[test]
9898
fn test_wait_ptrace() {
9999
require_capability!("test_wait_ptrace", CAP_SYS_PTRACE);
100-
let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
100+
let _m = crate::FORK_MTX.lock();
101101

102102
match unsafe{fork()}.expect("Error: Fork Failed") {
103103
Child => ptrace_child(),

test/test.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ mod test_unistd;
4343

4444
use std::os::unix::io::RawFd;
4545
use std::path::PathBuf;
46-
use std::sync::{Mutex, RwLock, RwLockWriteGuard};
46+
use parking_lot::{Mutex, RwLock, RwLockWriteGuard};
4747
use nix::unistd::{chdir, getcwd, read};
4848

4949

@@ -84,8 +84,7 @@ struct DirRestore<'a> {
8484

8585
impl<'a> DirRestore<'a> {
8686
fn new() -> Self {
87-
let guard = crate::CWD_LOCK.write()
88-
.expect("Lock got poisoned by another test");
87+
let guard = crate::CWD_LOCK.write();
8988
DirRestore{
9089
_g: guard,
9190
d: getcwd().unwrap(),

test/test_kmod/mod.rs

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@ use tempfile::{tempdir, TempDir};
55
use crate::*;
66

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

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

@@ -41,8 +39,8 @@ use std::io::Read;
4139
#[test]
4240
fn test_finit_and_delete_module() {
4341
require_capability!("test_finit_and_delete_module", CAP_SYS_MODULE);
44-
let _m0 = crate::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
45-
let _m1 = crate::CWD_LOCK.read().expect("Mutex got poisoned by another test");
42+
let _m0 = crate::KMOD_MTX.lock();
43+
let _m1 = crate::CWD_LOCK.read();
4644

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

@@ -59,8 +57,8 @@ fn test_finit_and_delete_module() {
5957
#[test]
6058
fn test_finit_and_delete_module_with_params() {
6159
require_capability!("test_finit_and_delete_module_with_params", CAP_SYS_MODULE);
62-
let _m0 = crate::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
63-
let _m1 = crate::CWD_LOCK.read().expect("Mutex got poisoned by another test");
60+
let _m0 = crate::KMOD_MTX.lock();
61+
let _m1 = crate::CWD_LOCK.read();
6462

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

@@ -80,8 +78,8 @@ fn test_finit_and_delete_module_with_params() {
8078
#[test]
8179
fn test_init_and_delete_module() {
8280
require_capability!("test_init_and_delete_module", CAP_SYS_MODULE);
83-
let _m0 = crate::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
84-
let _m1 = crate::CWD_LOCK.read().expect("Mutex got poisoned by another test");
81+
let _m0 = crate::KMOD_MTX.lock();
82+
let _m1 = crate::CWD_LOCK.read();
8583

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

@@ -100,8 +98,8 @@ fn test_init_and_delete_module() {
10098
#[test]
10199
fn test_init_and_delete_module_with_params() {
102100
require_capability!("test_init_and_delete_module_with_params", CAP_SYS_MODULE);
103-
let _m0 = crate::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
104-
let _m1 = crate::CWD_LOCK.read().expect("Mutex got poisoned by another test");
101+
let _m0 = crate::KMOD_MTX.lock();
102+
let _m1 = crate::CWD_LOCK.read();
105103

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

@@ -121,8 +119,8 @@ fn test_init_and_delete_module_with_params() {
121119
#[test]
122120
fn test_finit_module_invalid() {
123121
require_capability!("test_finit_module_invalid", CAP_SYS_MODULE);
124-
let _m0 = crate::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
125-
let _m1 = crate::CWD_LOCK.read().expect("Mutex got poisoned by another test");
122+
let _m0 = crate::KMOD_MTX.lock();
123+
let _m1 = crate::CWD_LOCK.read();
126124

127125
let kmod_path = "/dev/zero";
128126

@@ -135,8 +133,8 @@ fn test_finit_module_invalid() {
135133
#[test]
136134
fn test_finit_module_twice_and_delete_module() {
137135
require_capability!("test_finit_module_twice_and_delete_module", CAP_SYS_MODULE);
138-
let _m0 = crate::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
139-
let _m1 = crate::CWD_LOCK.read().expect("Mutex got poisoned by another test");
136+
let _m0 = crate::KMOD_MTX.lock();
137+
let _m1 = crate::CWD_LOCK.read();
140138

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

@@ -157,8 +155,8 @@ fn test_finit_module_twice_and_delete_module() {
157155
#[test]
158156
fn test_delete_module_not_loaded() {
159157
require_capability!("test_delete_module_not_loaded", CAP_SYS_MODULE);
160-
let _m0 = crate::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
161-
let _m1 = crate::CWD_LOCK.read().expect("Mutex got poisoned by another test");
158+
let _m0 = crate::KMOD_MTX.lock();
159+
let _m1 = crate::CWD_LOCK.read();
162160

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

test/test_pty.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ fn test_explicit_close() {
2929
#[test]
3030
#[cfg(any(target_os = "android", target_os = "linux"))]
3131
fn test_ptsname_equivalence() {
32-
let _m = crate::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
32+
let _m = crate::PTSNAME_MTX.lock();
3333

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

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

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

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

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

116116
#[cfg(target_os = "illumos")]
117-
// TODO: rewrite using ioctl!
117+
// TODO: rewrite using ioctl!
118118
#[allow(clippy::comparison_chain)]
119119
{
120120
use libc::{ioctl, I_FIND, I_PUSH};
@@ -187,7 +187,7 @@ fn test_write_ptty_pair() {
187187
#[test]
188188
fn test_openpty() {
189189
// openpty uses ptname(3) internally
190-
let _m = crate::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
190+
let _m = crate::PTSNAME_MTX.lock();
191191

192192
let pty = openpty(None, None).unwrap();
193193
assert!(pty.master > 0);
@@ -222,7 +222,7 @@ fn test_openpty() {
222222
#[test]
223223
fn test_openpty_with_termios() {
224224
// openpty uses ptname(3) internally
225-
let _m = crate::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
225+
let _m = crate::PTSNAME_MTX.lock();
226226

227227
// Open one pty to get attributes for the second one
228228
let mut termios = {
@@ -273,9 +273,9 @@ fn test_forkpty() {
273273
use nix::sys::signal::*;
274274
use nix::sys::wait::wait;
275275
// forkpty calls openpty which uses ptname(3) internally.
276-
let _m0 = crate::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
276+
let _m0 = crate::PTSNAME_MTX.lock();
277277
// forkpty spawns a child process
278-
let _m1 = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");
278+
let _m1 = crate::FORK_MTX.lock();
279279

280280
let string = "naninani\n";
281281
let echoed_string = "naninani\r\n";

0 commit comments

Comments
 (0)