search

stemjail / tty-rs

Pseudoterminal Rust library
https://stemjail.github.io/rustdoc/tty/
GNU General Public License v3.0
29 stars 9 forks source link

Fix race in TtyServer::spawn() #5

Closed xobs closed 7 years ago

xobs commented 7 years ago

This two-patch series fixes a problem I was seeing where occasionally, handles would be closed before they're used. This only occurred when there were two instances of a thread running, which is why there are two test functions.

The first patch adds a pair of tests that exhibit the problem. The second patch fixes spawn() so that it holds onto the handle long enough.

I have verified that the handles are correct using the following println inside spawn()

println!("Slave fd: {} Master fd: {} {:?} {:?}", slave.as_raw_fd(), self.get_master().as_raw_fd(), self.as_ref(), cmd);

xobs commented 7 years ago

I've also tested this in a shell with a limit of 32 filehandles, to make sure that it's cleaning up after itself despite setting "false" in the FileDesc. 16 filehandles was too few, but with "ulimit -n 32" both tests were able to run 10,000 times each without a failure.

xobs commented 7 years ago

This bug exists, but perhaps this isn't the best way to solve it. The testcase is good to have, but the second patch is not very high quality.