From 3fe0ba9afc7504ec01a778d8d72bd0b72fd013e1 Mon Sep 17 00:00:00 2001 From: Ivan Petkov Date: Sun, 10 Aug 2014 14:10:34 -0700 Subject: [PATCH] libnative: process spawning should not close inherited file descriptors * The caller should be responsible for cleaning up file descriptors * If a caller safely creates a file descriptor (via native::io::file::open) the returned structure (FileDesc) will try to clean up the file, failing in the process and writing error messages to the screen. * This should not happen as the caller has no public interface for telling the FileDesc structure to NOT free the underlying fd. * Alternatively, if another file is opened under the same fd held by the FileDesc structure returned by native::io::file::open, it will close the wrong file upon destruction. --- src/libnative/io/process.rs | 2 +- src/libstd/io/process.rs | 25 ++++++++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/libnative/io/process.rs b/src/libnative/io/process.rs index 77822bbbc20ff..4b832a4a97efa 100644 --- a/src/libnative/io/process.rs +++ b/src/libnative/io/process.rs @@ -80,7 +80,7 @@ impl Process { rtio::Ignored => { ret.push(None); Ok(None) } rtio::InheritFd(fd) => { ret.push(None); - Ok(Some(file::FileDesc::new(fd, true))) + Ok(Some(file::FileDesc::new(fd, false))) } rtio::CreatePipe(readable, _writable) => { let (reader, writer) = try!(pipe()); diff --git a/src/libstd/io/process.rs b/src/libstd/io/process.rs index 1eee69834948f..c82b4831341ef 100644 --- a/src/libstd/io/process.rs +++ b/src/libstd/io/process.rs @@ -378,7 +378,8 @@ pub enum StdioContainer { Ignored, /// The specified file descriptor is inherited for the stream which it is - /// specified for. + /// specified for. Ownership of the file descriptor is *not* taken, so the + /// caller must clean it up. InheritFd(libc::c_int), /// Creates a pipe for the specified file descriptor which will be created @@ -605,6 +606,7 @@ impl Drop for Process { #[cfg(test)] mod tests { + extern crate native; use io::process::{Command, Process}; use prelude::*; @@ -1017,4 +1019,25 @@ mod tests { assert!(Process::kill(id, 0).is_ok()); assert!(Process::kill(id, PleaseExitSignal).is_ok()); }) + + iotest!(fn dont_close_fd_on_command_spawn() { + use std::rt::rtio::{Truncate, Write}; + use native::io::file; + + let path = if cfg!(windows) { + Path::new("NUL") + } else { + Path::new("/dev/null") + }; + + let mut fdes = match file::open(&path.to_c_str(), Truncate, Write) { + Ok(f) => f, + Err(_) => fail!("failed to open file descriptor"), + }; + + let mut cmd = pwd_cmd(); + let _ = cmd.stdout(InheritFd(fdes.fd())); + assert!(cmd.status().unwrap().success()); + assert!(fdes.inner_write("extra write\n".as_bytes()).is_ok()); + }) }