Merge #1076

1076: Miscellaneous test fixes r=asomers a=asomers

Miscellaneous fixes needed to get the tests working more reliably when --test-threads > 1 , or when run inside of Linux containers.

Co-authored-by: Alan Somers <asomers@gmail.com>

12 files changed, 159 insertions, 138 deletions

diff --git a/.cirrus.yml b/.cirrus.yml
index e8fddcfd..00d0095c 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -7,6 +7,9 @@ freebsd_instance:
 # the system's binaries, so the environment shouldn't matter.
 task:
   name: FreeBSD 11.2
+  cargo_cache:
+    folder: $CARGO_HOME/registry
+    fingerprint_script: cat Cargo.lock || echo ""
   # Install Rust
   setup_script:
     - pkg install -y curl
@@ -19,3 +22,4 @@ task:
   i386_test_script:
     - . $HOME/.cargo/env
     - cargo test --target i686-unknown-freebsd
+  before_cache_script: rm -rf $CARGO_HOME/registry/index
diff --git a/.travis.yml b/.travis.yml
index de68f214..bd14c30e 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -11,22 +11,19 @@ env:
     - RUST_TEST_THREADS=1
 
 matrix:
-  # These are all the build jobs. Adjust as necessary. Comment out what you
-  # don't need. The iOS/Mac builds are distributed throughout because Travis
-  # likes to have a big backlog on builds on those machines. This way at least
-  # all of the other jobs can finish while waiting on those builds.
   include:
     # iOS builds
     # These are all done on a single machine because Travis rations their OS X
-    # builders so heavily that we otherwise can't merge PRs during the work week.
-    # Additionally they're moved to the front of the line to get them in the Travis
-    # OS X build queue first.
+    # builders so heavily that we otherwise can't merge PRs during the work
+    # week.  Additionally they're moved to the front of the line to get them in
+    # the Travis OS X build queue first.
     - env: TARGET="aarch64-apple-ios;armv7-apple-ios;armv7s-apple-ios;i386-apple-ios;x86_64-apple-ios" DISABLE_TESTS=1
       rust: 1.24.1
       os: osx
 
     # Mac builds
-    # These are also moved to be first because they wait in a long queue with Travis
+    # These are also moved to be first because they wait in a long queue with
+    # Travis
     - env: TARGET=i686-apple-darwin
       rust: 1.24.1
       os: osx
@@ -81,9 +78,7 @@ matrix:
       rust: 1.24.1
 
     # *BSD
-    # FreeBSD i686 and x86_64 use BuildBot instead of Travis
-    # Note that i686-unknown-freebsd is actually using stable Rust instead of
-    # 1.13.0 because rustup does not support i686-unknown-freebsd
+    # FreeBSD i686 and x86_64 use Cirrus instead of Travis
     # - env: TARGET=i686-unknown-freebsd DISABLE_TESTS=1
     # - env: TARGET=x86_64-unknown-freebsd DISABLE_TESTS=1
     - env: TARGET=x86_64-unknown-netbsd DISABLE_TESTS=1
@@ -104,30 +99,6 @@ script:
 
 after_script: set +e
 
-before_deploy:
-  - sh ci/before_deploy.sh
-
-deploy:
-  # TODO update `api_key.secure`
-  # - Create a `public_repo` GitHub token. Go to: https://github.com/settings/tokens/new
-  # - Encrypt it: `travis encrypt GH_TOKEN=0123456789012345678901234567890123456789`
-  # - Paste the output down here
-  api_key:
-    secure: S1ktt0eqmfrEHnYPf4WO7mZtatz/FWfYWBp8nwdc0nd8H6UNZ9Dwy3tJpVe0N9rpB9vAFnkdw6R4jdkIcgxfory2F3F8k/mh8cWn0mkvh2N34YjHMYLnuVzOoFrWai7IcPfROpdlY0tGBlwNj5KMkeBnHUJzd2q4j/4j/tlrfmg=
-  file_glob: true
-  file: $CRATE_NAME-$TRAVIS_TAG-$TARGET.*
-  on:
-    # Here you can pick which targets will generate binary releases
-    # In this example, there are some targets that are tested using the stable
-    # and nightly channels. This condition makes sure there is only one release
-    # for such targets and that's generated using the stable channel
-    #
-    # Here we make it so we never generate binary releases
-    condition: $DEPLOY = never
-    tags: true
-  provider: releases
-  skip_cleanup: true
-
 cache: cargo
 before_cache:
   # Travis can't cache files that are not readable by "others"
diff --git a/Cargo.toml b/Cargo.toml
index 2f734aec..f8e4bee9 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -27,9 +27,12 @@ cc = "1"
 [dev-dependencies]
 bytes = "0.4.8"
 lazy_static = "1.2"
-rand = "0.6"
+rand = ">= 0.6, < 0.7"
 tempfile = "3.0.5"
 
+[target.'cfg(any(target_os = "android", target_os = "linux"))'.dev-dependencies]
+caps = "0.3.1"
+
 [target.'cfg(target_os = "freebsd")'.dev-dependencies]
 sysctl = "0.1"
 
diff --git a/ci/before_deploy.ps1 b/ci/before_deploy.ps1
deleted file mode 100644
index 191a30b8..00000000
--- a/ci/before_deploy.ps1
+++ /dev/null
@@ -1,23 +0,0 @@
-# This script takes care of packaging the build artifacts that will go in the
-# release zipfile
-
-$SRC_DIR = $PWD.Path
-$STAGE = [System.Guid]::NewGuid().ToString()
-
-Set-Location $ENV:Temp
-New-Item -Type Directory -Name $STAGE
-Set-Location $STAGE
-
-$ZIP = "$SRC_DIR\$($Env:CRATE_NAME)-$($Env:APPVEYOR_REPO_TAG_NAME)-$($Env:TARGET).zip"
-
-# TODO Update this to package the right artifacts
-Copy-Item "$SRC_DIR\target\$($Env:TARGET)\release\hello.exe" '.\'
-
-7z a "$ZIP" *
-
-Push-AppveyorArtifact "$ZIP"
-
-Remove-Item *.* -Force
-Set-Location ..
-Remove-Item $STAGE
-Set-Location $SRC_DIR
diff --git a/ci/before_deploy.sh b/ci/before_deploy.sh
deleted file mode 100644
index 026dc289..00000000
--- a/ci/before_deploy.sh
+++ /dev/null
@@ -1,33 +0,0 @@
-# This script takes care of building your crate and packaging it for release
-
-set -ex
-
-main() {
-    local src=$(pwd) \
-          stage=
-
-    case $TRAVIS_OS_NAME in
-        linux)
-            stage=$(mktemp -d)
-            ;;
-        osx)
-            stage=$(mktemp -d -t tmp)
-            ;;
-    esac
-
-    test -f Cargo.lock || cargo generate-lockfile
-
-    # TODO Update this to build the artifacts that matter to you
-    cross rustc --bin hello --target $TARGET --release -- -C lto
-
-    # TODO Update this to package the right artifacts
-    cp target/$TARGET/release/hello $stage/
-
-    cd $stage
-    tar czf $src/$CRATE_NAME-$TRAVIS_TAG-$TARGET.tar.gz *
-    cd $src
-
-    rm -rf $stage
-}
-
-main
diff --git a/src/sys/socket/sockopt.rs b/src/sys/socket/sockopt.rs
index 4727698b..5f19e5f3 100644
--- a/src/sys/socket/sockopt.rs
+++ b/src/sys/socket/sockopt.rs
@@ -677,23 +677,4 @@ mod test {
         close(s).unwrap();
     }
 
-    #[cfg(target_os = "linux")]
-    #[test]
-    fn is_so_mark_functional() {
-        use super::super::*;
-        use ::unistd::Uid;
-        use ::std::io::{self, Write};
-
-        if !Uid::current().is_root() {
-            let stderr = io::stderr();
-            let mut handle = stderr.lock();
-            writeln!(handle, "SO_MARK requires root privileges. Skipping test.").unwrap();
-            return;
-        }
-
-        let s = socket(AddressFamily::Inet, SockType::Stream, SockFlag::empty(), None).unwrap();
-        setsockopt(s, super::Mark, &1337).unwrap();
-        let mark = getsockopt(s, super::Mark).unwrap();
-        assert_eq!(mark, 1337);
-    }
 }
diff --git a/test/sys/test_pthread.rs b/test/sys/test_pthread.rs
index 7e6baea0..89280100 100644
--- a/test/sys/test_pthread.rs
+++ b/test/sys/test_pthread.rs
@@ -1,11 +1,10 @@
 use nix::sys::pthread::*;
-use std::ptr;
 
 #[cfg(target_env = "musl")]
 #[test]
 fn test_pthread_self() {
     let tid = pthread_self();
-    assert!(tid != ptr::null_mut());
+    assert!(tid != ::std::ptr::null_mut());
 }
 
 #[cfg(not(target_env = "musl"))]
diff --git a/test/sys/test_sockopt.rs b/test/sys/test_sockopt.rs
index efe2c56b..c4860c0d 100644
--- a/test/sys/test_sockopt.rs
+++ b/test/sys/test_sockopt.rs
@@ -1,6 +1,19 @@
 use rand::{thread_rng, Rng};
 use nix::sys::socket::{socket, sockopt, getsockopt, setsockopt, AddressFamily, SockType, SockFlag, SockProtocol};
 
+#[cfg(target_os = "linux")]
+#[test]
+fn is_so_mark_functional() {
+    use nix::sys::socket::sockopt;
+
+    require_capability!(CAP_NET_ADMIN);
+
+    let s = socket(AddressFamily::Inet, SockType::Stream, SockFlag::empty(), None).unwrap();
+    setsockopt(s, sockopt::Mark, &1337).unwrap();
+    let mark = getsockopt(s, sockopt::Mark).unwrap();
+    assert_eq!(mark, 1337);
+}
+
 #[test]
 fn test_so_buf() {
     let fd = socket(AddressFamily::Inet, SockType::Datagram, SockFlag::empty(), SockProtocol::Udp)
diff --git a/test/test.rs b/test/test.rs
index a91b6348..4c7e5b1e 100644
--- a/test/test.rs
+++ b/test/test.rs
@@ -1,4 +1,6 @@
 extern crate bytes;
+#[cfg(any(target_os = "android", target_os = "linux"))]
+extern crate caps;
 #[macro_use]
 extern crate cfg_if;
 #[macro_use]
@@ -7,16 +9,51 @@ extern crate nix;
 extern crate lazy_static;
 extern crate libc;
 extern crate rand;
+#[cfg(target_os = "freebsd")]
+extern crate sysctl;
 extern crate tempfile;
 
+#[cfg(any(target_os = "android", target_os = "linux"))]
+macro_rules! require_capability {
+    ($capname:ident) => {
+        use ::caps::{Capability, CapSet, has_cap};
+        use ::std::io::{self, Write};
+
+        if !has_cap(None, CapSet::Effective, Capability::$capname).unwrap() {
+            let stderr = io::stderr();
+            let mut handle = stderr.lock();
+            writeln!(handle, "Insufficient capabilities. Skipping test.")
+                .unwrap();
+            return;
+        }
+    }
+}
+
+#[cfg(target_os = "freebsd")]
+macro_rules! skip_if_jailed {
+    ($name:expr) => {
+        use ::sysctl::CtlValue;
+
+        if let CtlValue::Int(1) = ::sysctl::value("security.jail.jailed")
+            .unwrap()
+        {
+            use ::std::io::Write;
+            let stderr = ::std::io::stderr();
+            let mut handle = stderr.lock();
+            writeln!(handle, "{} cannot run in a jail. Skipping test.", $name)
+                .unwrap();
+            return;
+        }
+    }
+}
+
 macro_rules! skip_if_not_root {
     ($name:expr) => {
         use nix::unistd::Uid;
-        use std;
-        use std::io::Write;
 
         if !Uid::current().is_root() {
-            let stderr = std::io::stderr();
+            use ::std::io::Write;
+            let stderr = ::std::io::stderr();
             let mut handle = stderr.lock();
             writeln!(handle, "{} requires root privileges. Skipping test.", $name).unwrap();
             return;
@@ -50,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]) {
@@ -65,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 e0cb0df5..ad406357 100644
--- a/test/test_kmod/mod.rs
+++ b/test/test_kmod/mod.rs
@@ -40,7 +40,9 @@ use std::io::Read;
 
 #[test]
 fn test_finit_and_delete_module() {
-    skip_if_not_root!("test_finit_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();
 
@@ -56,7 +58,9 @@ fn test_finit_and_delete_module() {
 
 #[test]
 fn test_finit_and_delete_modul_with_params() {
-    skip_if_not_root!("test_finit_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();
 
@@ -75,7 +79,9 @@ fn test_finit_and_delete_modul_with_params() {
 
 #[test]
 fn test_init_and_delete_module() {
-    skip_if_not_root!("test_init_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();
 
@@ -93,7 +99,9 @@ fn test_init_and_delete_module() {
 
 #[test]
 fn test_init_and_delete_module_with_params() {
-    skip_if_not_root!("test_init_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();
 
@@ -112,7 +120,9 @@ fn test_init_and_delete_module_with_params() {
 
 #[test]
 fn test_finit_module_invalid() {
-    skip_if_not_root!("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";
 
@@ -124,7 +134,9 @@ fn test_finit_module_invalid() {
 
 #[test]
 fn test_finit_module_twice_and_delete_module() {
-    skip_if_not_root!("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();
 
@@ -144,7 +156,9 @@ fn test_finit_module_twice_and_delete_module() {
 
 #[test]
 fn test_delete_module_not_loaded() {
-    skip_if_not_root!("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 3f13883c..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());
@@ -387,28 +388,50 @@ fn test_lseek64() {
     close(tmpfd).unwrap();
 }
 
-// Skip on FreeBSD because FreeBSD's CI environment is jailed, and jails
-// aren't allowed to use acct(2)
-#[cfg(not(target_os = "freebsd"))]
+cfg_if!{
+    if #[cfg(any(target_os = "android", target_os = "linux"))] {
+        macro_rules! require_acct{
+            () => {
+                require_capability!(CAP_SYS_PACCT);
+            }
+        }
+    } else if #[cfg(target_os = "freebsd")] {
+        macro_rules! require_acct{
+            () => {
+                skip_if_not_root!("test_acct");
+                skip_if_jailed!("test_acct");
+            }
+        }
+    } else {
+        macro_rules! require_acct{
+            () => {
+                skip_if_not_root!("test_acct");
+            }
+        }
+    }
+}
+
 #[test]
 fn test_acct() {
     use tempfile::NamedTempFile;
     use std::process::Command;
     use std::{thread, time};
 
-    skip_if_not_root!("test_acct");
+    let _m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+    require_acct!();
+
     let file = NamedTempFile::new().unwrap();
     let path = file.path().to_str().unwrap();
 
     acct::enable(path).unwrap();
-    Command::new("echo").arg("Hello world");
-    acct::disable().unwrap();
 
     loop {
+        Command::new("echo").arg("Hello world");
         let len = fs::metadata(path).unwrap().len();
         if len > 0 { break; }
         thread::sleep(time::Duration::from_millis(10));
     }
+    acct::disable().unwrap();
 }
 
 #[test]