From 5fb4cebcc6c9b241668d36c8732c93e1978b135e Mon Sep 17 00:00:00 2001
From: Alan Somers <asomers@gmail.com>
Date: Sat, 15 Jul 2017 10:47:58 -0600
Subject: PtyMaster::drop should panic on EBADF

Also, document the double-close risk with unistd::close

Fixes #659
---
 test/test_pty.rs            | 16 ++++++++++++++++
 test/test_ptymaster_drop.rs | 21 +++++++++++++++++++++
 2 files changed, 37 insertions(+)
 create mode 100644 test/test_ptymaster_drop.rs

(limited to 'test')

diff --git a/test/test_pty.rs b/test/test_pty.rs
index 53e94724..55316ab4 100644
--- a/test/test_pty.rs
+++ b/test/test_pty.rs
@@ -1,5 +1,7 @@
+use std::io::Write;
 use std::path::Path;
 use std::os::unix::prelude::*;
+use tempfile::tempfile;
 
 use nix::fcntl::{O_RDWR, open};
 use nix::pty::*;
@@ -7,6 +9,20 @@ use nix::sys::stat;
 use nix::sys::termios::*;
 use nix::unistd::{write, close};
 
+/// Regression test for Issue #659
+/// This is the correct way to explicitly close a PtyMaster
+#[test]
+fn test_explicit_close() {
+    let mut f = {
+        let m = posix_openpt(O_RDWR).unwrap();
+        close(m.into_raw_fd()).unwrap();
+        tempfile().unwrap()
+    };
+    // This should work.  But if there's been a double close, then it will
+    // return EBADF
+    f.write(b"whatever").unwrap();
+}
+
 /// Test equivalence of `ptsname` and `ptsname_r`
 #[test]
 #[cfg(any(target_os = "android", target_os = "linux"))]
diff --git a/test/test_ptymaster_drop.rs b/test/test_ptymaster_drop.rs
new file mode 100644
index 00000000..664a4dd6
--- /dev/null
+++ b/test/test_ptymaster_drop.rs
@@ -0,0 +1,21 @@
+extern crate nix;
+
+use nix::fcntl::O_RDWR;
+use nix::pty::*;
+use nix::unistd::close;
+use std::os::unix::io::AsRawFd;
+
+/// Regression test for Issue #659
+/// PtyMaster should panic rather than double close the file descriptor
+/// This must run in its own test process because it deliberately creates a race
+/// condition.
+#[test]
+#[should_panic(expected = "Closing an invalid file descriptor!")]
+// In Travis on i686-unknown-linux-musl, this test gets SIGABRT.  I don't know
+// why.  It doesn't happen on any other target, and it doesn't happen on my PC.
+#[cfg_attr(all(target_env = "musl", target_arch = "x86"), ignore)]
+fn test_double_close() {
+    let m = posix_openpt(O_RDWR).unwrap();
+    close(m.as_raw_fd()).unwrap();
+    drop(m);            // should panic here
+}
-- 
cgit v1.2.3