Merge pull request #691 from asomers/ptsname

Fix thread safety issues in pty and termios tests

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();

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(());
 }

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();