From ce04f1ce80dc3109ae27fbe5cefa6cdb52625b52 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Wed, 19 Jul 2017 22:59:47 -0600 Subject: Fix thread safety issues in pty and termios tests ptsname(3) returns a pointer to a global variable, so it isn't thread-safe. Protect it with a mutex. --- test/sys/test_termios.rs | 22 +++++++++++++++++----- test/test.rs | 2 ++ test/test_pty.rs | 28 ++++++++++++++++++++++++---- 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/test/sys/test_termios.rs b/test/sys/test_termios.rs index c6572052..2455a4e5 100644 --- a/test/sys/test_termios.rs +++ b/test/sys/test_termios.rs @@ -18,10 +18,14 @@ fn write_all(f: RawFd, buf: &[u8]) { // Test tcgetattr on a terminal #[test] fn test_tcgetattr_pty() { - let pty = openpty(None, None).unwrap(); + // openpty uses ptname(3) internally + #[allow(unused_variables)] + let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test"); + + let pty = openpty(None, None).expect("openpty failed"); assert!(termios::tcgetattr(pty.master).is_ok()); - close(pty.master).unwrap(); - close(pty.slave).unwrap(); + close(pty.master).expect("closing the master failed"); + close(pty.slave).expect("closing the slave failed"); } // Test tcgetattr on something that isn't a terminal #[test] @@ -41,12 +45,16 @@ fn test_tcgetattr_ebadf() { // Test modifying output flags #[test] fn test_output_flags() { + // openpty uses ptname(3) internally + #[allow(unused_variables)] + let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test"); + // Open one pty to get attributes for the second one let mut termios = { - let pty = openpty(None, None).unwrap(); + let pty = openpty(None, None).expect("openpty failed"); assert!(pty.master > 0); assert!(pty.slave > 0); - let termios = tcgetattr(pty.master).unwrap(); + let termios = tcgetattr(pty.master).expect("tcgetattr failed"); close(pty.master).unwrap(); close(pty.slave).unwrap(); termios @@ -80,6 +88,10 @@ fn test_output_flags() { // Test modifying local flags #[test] fn test_local_flags() { + // openpty uses ptname(3) internally + #[allow(unused_variables)] + let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test"); + // Open one pty to get attributes for the second one let mut termios = { let pty = openpty(None, None).unwrap(); diff --git a/test/test.rs b/test/test.rs index eb4556fa..fab499ce 100644 --- a/test/test.rs +++ b/test/test.rs @@ -45,6 +45,8 @@ lazy_static! { /// 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 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(()); } diff --git a/test/test_pty.rs b/test/test_pty.rs index 55316ab4..75ef4923 100644 --- a/test/test_pty.rs +++ b/test/test_pty.rs @@ -27,6 +27,9 @@ fn test_explicit_close() { #[test] #[cfg(any(target_os = "android", target_os = "linux"))] fn test_ptsname_equivalence() { + #[allow(unused_variables)] + let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test"); + // Open a new PTTY master let master_fd = posix_openpt(O_RDWR).unwrap(); assert!(master_fd.as_raw_fd() > 0); @@ -42,6 +45,9 @@ fn test_ptsname_equivalence() { #[test] #[cfg(any(target_os = "android", target_os = "linux"))] fn test_ptsname_copy() { + #[allow(unused_variables)] + let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test"); + // Open a new PTTY master let master_fd = posix_openpt(O_RDWR).unwrap(); assert!(master_fd.as_raw_fd() > 0); @@ -74,6 +80,9 @@ fn test_ptsname_r_copy() { #[test] #[cfg(any(target_os = "android", target_os = "linux"))] fn test_ptsname_unique() { + #[allow(unused_variables)] + let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test"); + // Open a new PTTY master let master1_fd = posix_openpt(O_RDWR).unwrap(); assert!(master1_fd.as_raw_fd() > 0); @@ -95,16 +104,19 @@ fn test_ptsname_unique() { /// pair. #[test] fn test_open_ptty_pair() { + #[allow(unused_variables)] + let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test"); + // Open a new PTTY master - let master_fd = posix_openpt(O_RDWR).unwrap(); + let master_fd = posix_openpt(O_RDWR).expect("posix_openpt failed"); assert!(master_fd.as_raw_fd() > 0); // Allow a slave to be generated for it - grantpt(&master_fd).unwrap(); - unlockpt(&master_fd).unwrap(); + grantpt(&master_fd).expect("grantpt failed"); + unlockpt(&master_fd).expect("unlockpt failed"); // Get the name of the slave - let slave_name = ptsname(&master_fd).unwrap(); + let slave_name = ptsname(&master_fd).expect("ptsname failed"); // Open the slave device let slave_fd = open(Path::new(&slave_name), O_RDWR, stat::Mode::empty()).unwrap(); @@ -113,6 +125,10 @@ fn test_open_ptty_pair() { #[test] fn test_openpty() { + // openpty uses ptname(3) internally + #[allow(unused_variables)] + let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test"); + let pty = openpty(None, None).unwrap(); assert!(pty.master > 0); assert!(pty.slave > 0); @@ -145,6 +161,10 @@ fn test_openpty() { #[test] fn test_openpty_with_termios() { + // openpty uses ptname(3) internally + #[allow(unused_variables)] + let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test"); + // Open one pty to get attributes for the second one let mut termios = { let pty = openpty(None, None).unwrap(); -- cgit v1.2.3