Skip to content

Commit

Permalink
Fix thread safety issues in pty and termios tests
Browse files Browse the repository at this point in the history
ptsname(3) returns a pointer to a global variable, so it isn't
thread-safe.  Protect it with a mutex.
  • Loading branch information
asomers committed Jul 20, 2017
1 parent 07e6c2f commit ce04f1c
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 9 deletions.
22 changes: 17 additions & 5 deletions test/sys/test_termios.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions test/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(());
}
Expand Down
28 changes: 24 additions & 4 deletions test/test_pty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit ce04f1c

Please sign in to comment.