Fix some bugs with multithreaded tests:

* kmod tests must run exclusively, because they load and unload a module
  with a constant name.
* A few tests were doing some variant of chdir, but weren't taking the
  CWD_MTX.
* The kmod tests read files by path relative to CWD, so they need the
  CWD_MTX.  But they don't need it exclusively, so convert the CWD_MTX
  into an RwLock.
* Tests that do change the cwd need to change it back when they're done.

4 files changed, 56 insertions, 9 deletions

diff --git a/test/test.rs b/test/test.rs
index c8b8717f..4c7e5b1e 100644
--- a/test/test.rs
+++ b/test/test.rs
@@ -87,8 +87,9 @@ mod test_stat;
 mod test_unistd;
 
 use std::os::unix::io::RawFd;
-use std::sync::Mutex;
-use nix::unistd::read;
+use std::path::PathBuf;
+use std::sync::{Mutex, RwLock, RwLockWriteGuard};
+use nix::unistd::{chdir, getcwd, read};
 
 /// Helper function analogous to `std::io::Read::read_exact`, but for `RawFD`s
 fn read_exact(f: RawFd, buf: &mut  [u8]) {
@@ -102,16 +103,45 @@ fn read_exact(f: RawFd, buf: &mut  [u8]) {
 
 lazy_static! {
     /// Any test that changes the process's current working directory must grab
-    /// this mutex
-    pub static ref CWD_MTX: Mutex<()> = Mutex::new(());
-    /// Any test that changes the process's supplementary groups must grab this
-    /// mutex
-    pub static ref GROUPS_MTX: Mutex<()> = Mutex::new(());
+    /// the RwLock exclusively.  Any process that cares about the current
+    /// working directory must grab it shared.
+    pub static ref CWD_LOCK: RwLock<()> = RwLock::new(());
     /// Any test that creates child processes must grab this mutex, regardless
     /// of what it does with those children.
     pub static ref FORK_MTX: Mutex<()> = Mutex::new(());
+    /// Any test that changes the process's supplementary groups must grab this
+    /// mutex
+    pub static ref GROUPS_MTX: Mutex<()> = Mutex::new(());
+    /// Any tests that loads or unloads kernel modules must grab this mutex
+    pub static ref KMOD_MTX: Mutex<()> = Mutex::new(());
     /// Any test that calls ptsname(3) must grab this mutex.
     pub static ref PTSNAME_MTX: Mutex<()> = Mutex::new(());
     /// Any test that alters signal handling must grab this mutex.
     pub static ref SIGNAL_MTX: Mutex<()> = Mutex::new(());
 }
+
+/// RAII object that restores a test's original directory on drop
+struct DirRestore<'a> {
+    d: PathBuf,
+    _g: RwLockWriteGuard<'a, ()>
+}
+
+impl<'a> DirRestore<'a> {
+    fn new() -> Self {
+        let guard = ::CWD_LOCK.write()
+            .expect("Lock got poisoned by another test");
+        DirRestore{
+            _g: guard,
+            d: getcwd().unwrap(),
+        }
+    }
+}
+
+impl<'a> Drop for DirRestore<'a> {
+    fn drop(&mut self) {
+        let r = chdir(&self.d);
+        if std::thread::panicking() {
+            r.unwrap();
+        }
+    }
+}
diff --git a/test/test_kmod/mod.rs b/test/test_kmod/mod.rs
index 86558d1b..ad406357 100644
--- a/test/test_kmod/mod.rs
+++ b/test/test_kmod/mod.rs
@@ -41,6 +41,8 @@ use std::io::Read;
 #[test]
 fn test_finit_and_delete_module() {
     require_capability!(CAP_SYS_MODULE);
+    let _m0 = ::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m1 = ::CWD_LOCK.read().expect("Mutex got poisoned by another test");
 
     let (kmod_path, kmod_name, _kmod_dir) = compile_kernel_module();
 
@@ -57,6 +59,8 @@ fn test_finit_and_delete_module() {
 #[test]
 fn test_finit_and_delete_modul_with_params() {
     require_capability!(CAP_SYS_MODULE);
+    let _m0 = ::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m1 = ::CWD_LOCK.read().expect("Mutex got poisoned by another test");
 
     let (kmod_path, kmod_name, _kmod_dir) = compile_kernel_module();
 
@@ -76,6 +80,8 @@ fn test_finit_and_delete_modul_with_params() {
 #[test]
 fn test_init_and_delete_module() {
     require_capability!(CAP_SYS_MODULE);
+    let _m0 = ::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m1 = ::CWD_LOCK.read().expect("Mutex got poisoned by another test");
 
     let (kmod_path, kmod_name, _kmod_dir) = compile_kernel_module();
 
@@ -94,6 +100,8 @@ fn test_init_and_delete_module() {
 #[test]
 fn test_init_and_delete_module_with_params() {
     require_capability!(CAP_SYS_MODULE);
+    let _m0 = ::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m1 = ::CWD_LOCK.read().expect("Mutex got poisoned by another test");
 
     let (kmod_path, kmod_name, _kmod_dir) = compile_kernel_module();
 
@@ -113,6 +121,8 @@ fn test_init_and_delete_module_with_params() {
 #[test]
 fn test_finit_module_invalid() {
     require_capability!(CAP_SYS_MODULE);
+    let _m0 = ::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m1 = ::CWD_LOCK.read().expect("Mutex got poisoned by another test");
 
     let kmod_path = "/dev/zero";
 
@@ -125,6 +135,8 @@ fn test_finit_module_invalid() {
 #[test]
 fn test_finit_module_twice_and_delete_module() {
     require_capability!(CAP_SYS_MODULE);
+    let _m0 = ::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m1 = ::CWD_LOCK.read().expect("Mutex got poisoned by another test");
 
     let (kmod_path, kmod_name, _kmod_dir) = compile_kernel_module();
 
@@ -145,6 +157,8 @@ fn test_finit_module_twice_and_delete_module() {
 #[test]
 fn test_delete_module_not_loaded() {
     require_capability!(CAP_SYS_MODULE);
+    let _m0 = ::KMOD_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m1 = ::CWD_LOCK.read().expect("Mutex got poisoned by another test");
 
     let result = delete_module(&CString::new("hello").unwrap(), DeleteModuleFlags::empty());
 
diff --git a/test/test_stat.rs b/test/test_stat.rs
index fae8df82..b9da7fc3 100644
--- a/test/test_stat.rs
+++ b/test/test_stat.rs
@@ -154,6 +154,7 @@ fn test_fchmod() {
 
 #[test]
 fn test_fchmodat() {
+    let _dr = ::DirRestore::new();
     let tempdir = tempfile::tempdir().unwrap();
     let filename = "foo.txt";
     let fullpath = tempdir.path().join(filename);
@@ -241,6 +242,7 @@ fn test_futimens() {
 
 #[test]
 fn test_utimensat() {
+    let _dr = ::DirRestore::new();
     let tempdir = tempfile::tempdir().unwrap();
     let filename = "foo.txt";
     let fullpath = tempdir.path().join(filename);
diff --git a/test/test_unistd.rs b/test/test_unistd.rs
index 73ed845e..b2061458 100644
--- a/test/test_unistd.rs
+++ b/test/test_unistd.rs
@@ -274,7 +274,7 @@ cfg_if!{
 #[test]
 fn test_fchdir() {
     // fchdir changes the process's cwd
-    let _m = ::CWD_MTX.lock().expect("Mutex got poisoned by another test");
+    let _dr = ::DirRestore::new();
 
     let tmpdir = tempfile::tempdir().unwrap();
     let tmpdir_path = tmpdir.path().canonicalize().unwrap();
@@ -289,7 +289,7 @@ fn test_fchdir() {
 #[test]
 fn test_getcwd() {
     // chdir changes the process's cwd
-    let _m = ::CWD_MTX.lock().expect("Mutex got poisoned by another test");
+    let _dr = ::DirRestore::new();
 
     let tmpdir = tempfile::tempdir().unwrap();
     let tmpdir_path = tmpdir.path().canonicalize().unwrap();
@@ -332,6 +332,7 @@ fn test_chown() {
 
 #[test]
 fn test_fchownat() {
+    let _dr = ::DirRestore::new();
     // Testing for anything other than our own UID/GID is hard.
     let uid = Some(getuid());
     let gid = Some(getgid());