Auto merge of #24034 - alexcrichton:cloexec, r=aturon

The commit messages have more details as to what's going on, but this is a breaking change for any libraries which expect file descriptors to be inherited by default.

Closes #12148

Author: bors

src/libstd/process.rs

@@ -19,13 +19,13 @@ use io::prelude::*;
 use ffi::OsStr;
 use fmt;
 use io::{self, Error, ErrorKind};
-use libc;
 use path;
 use sync::mpsc::{channel, Receiver};
 use sys::pipe2::{self, AnonPipe};
 use sys::process2::Command as CommandImp;
 use sys::process2::Process as ProcessImp;
 use sys::process2::ExitStatus as ExitStatusImp;
+use sys::process2::Stdio as StdioImp2;
 use sys_common::{AsInner, AsInnerMut};
 use thread;
 
@@ -229,13 +229,13 @@ impl Command {
 
     fn spawn_inner(&self, default_io: StdioImp) -> io::Result<Child> {
         let (their_stdin, our_stdin) = try!(
-            setup_io(self.stdin.as_ref().unwrap_or(&default_io), 0, true)
+            setup_io(self.stdin.as_ref().unwrap_or(&default_io), true)
         );
         let (their_stdout, our_stdout) = try!(
-            setup_io(self.stdout.as_ref().unwrap_or(&default_io), 1, false)
+            setup_io(self.stdout.as_ref().unwrap_or(&default_io), false)
         );
         let (their_stderr, our_stderr) = try!(
-            setup_io(self.stderr.as_ref().unwrap_or(&default_io), 2, false)
+            setup_io(self.stderr.as_ref().unwrap_or(&default_io), false)
         );
 
         match ProcessImp::spawn(&self.inner, their_stdin, their_stdout, their_stderr) {
@@ -328,23 +328,19 @@ impl AsInnerMut<CommandImp> for Command {
     fn as_inner_mut(&mut self) -> &mut CommandImp { &mut self.inner }
 }
 
-fn setup_io(io: &StdioImp, fd: libc::c_int, readable: bool)
-            -> io::Result<(Option<AnonPipe>, Option<AnonPipe>)>
+fn setup_io(io: &StdioImp, readable: bool)
+            -> io::Result<(StdioImp2, Option<AnonPipe>)>
 {
     use self::StdioImp::*;
     Ok(match *io {
-        Null => {
-            (None, None)
-        }
-        Inherit => {
-            (Some(AnonPipe::from_fd(fd)), None)
-        }
+        Null => (StdioImp2::None, None),
+        Inherit => (StdioImp2::Inherit, None),
         Piped => {
-            let (reader, writer) = try!(unsafe { pipe2::anon_pipe() });
+            let (reader, writer) = try!(pipe2::anon_pipe());
             if readable {
-                (Some(reader), Some(writer))
+                (StdioImp2::Piped(reader), Some(writer))
             } else {
-                (Some(writer), Some(reader))
+                (StdioImp2::Piped(writer), Some(reader))
             }
         }
     })

src/libstd/sys/unix/c.rs

@@ -26,39 +26,35 @@ use libc;
           target_os = "dragonfly",
           target_os = "bitrig",
           target_os = "openbsd"))]
-pub const FIONBIO: libc::c_ulong = 0x8004667e;
-#[cfg(any(all(target_os = "linux",
-              any(target_arch = "x86",
-                  target_arch = "x86_64",
-                  target_arch = "arm",
-                  target_arch = "aarch64")),
-          target_os = "android"))]
-pub const FIONBIO: libc::c_ulong = 0x5421;
-#[cfg(all(target_os = "linux",
-          any(target_arch = "mips",
-              target_arch = "mipsel",
-              target_arch = "powerpc")))]
-pub const FIONBIO: libc::c_ulong = 0x667e;
-
-#[cfg(any(target_os = "macos",
-          target_os = "ios",
-          target_os = "freebsd",
-          target_os = "dragonfly",
-          target_os = "bitrig",
-          target_os = "openbsd"))]
-pub const FIOCLEX: libc::c_ulong = 0x20006601;
+mod consts {
+    use libc;
+    pub const FIONBIO: libc::c_ulong = 0x8004667e;
+    pub const FIOCLEX: libc::c_ulong = 0x20006601;
+    pub const FIONCLEX: libc::c_ulong = 0x20006602;
+}
 #[cfg(any(all(target_os = "linux",
               any(target_arch = "x86",
                   target_arch = "x86_64",
                   target_arch = "arm",
                   target_arch = "aarch64")),
           target_os = "android"))]
-pub const FIOCLEX: libc::c_ulong = 0x5451;
+mod consts {
+    use libc;
+    pub const FIONBIO: libc::c_ulong = 0x5421;
+    pub const FIOCLEX: libc::c_ulong = 0x5451;
+    pub const FIONCLEX: libc::c_ulong = 0x5450;
+}
 #[cfg(all(target_os = "linux",
           any(target_arch = "mips",
               target_arch = "mipsel",
               target_arch = "powerpc")))]
-pub const FIOCLEX: libc::c_ulong = 0x6601;
+mod consts {
+    use libc;
+    pub const FIONBIO: libc::c_ulong = 0x667e;
+    pub const FIOCLEX: libc::c_ulong = 0x6601;
+    pub const FIONCLEX: libc::c_ulong = 0x6600;
+}
+pub use self::consts::*;
 
 #[cfg(any(target_os = "macos",
           target_os = "ios",
@@ -163,6 +159,8 @@ extern {
     pub fn utimes(filename: *const libc::c_char,
                   times: *const libc::timeval) -> libc::c_int;
     pub fn gai_strerror(errcode: libc::c_int) -> *const libc::c_char;
+    pub fn setgroups(ngroups: libc::c_int,
+                     ptr: *const libc::c_void) -> libc::c_int;
 }
 
 #[cfg(any(target_os = "macos", target_os = "ios"))]

src/libstd/sys/unix/fd.rs

@@ -13,6 +13,7 @@ use core::prelude::*;
 use io;
 use libc::{self, c_int, size_t, c_void};
 use mem;
+use sys::c;
 use sys::cvt;
 use sys_common::AsInner;
 
@@ -51,6 +52,13 @@ impl FileDesc {
         }));
         Ok(ret as usize)
     }
+
+    pub fn set_cloexec(&self) {
+        unsafe {
+            let ret = c::ioctl(self.fd, c::FIOCLEX);
+            debug_assert_eq!(ret, 0);
+        }
+    }
 }
 
 impl AsInner<c_int> for FileDesc {
@@ -59,14 +67,11 @@ impl AsInner<c_int> for FileDesc {
 
 impl Drop for FileDesc {
     fn drop(&mut self) {
-        // closing stdio file handles makes no sense, so never do it. Also, note
-        // that errors are ignored when closing a file descriptor. The reason
-        // for this is that if an error occurs we don't actually know if the
-        // file descriptor was closed or not, and if we retried (for something
-        // like EINTR), we might close another valid file descriptor (opened
-        // after we closed ours.
-        if self.fd > libc::STDERR_FILENO {
-            let _ = unsafe { libc::close(self.fd) };
-        }
+        // Note that errors are ignored when closing a file descriptor. The
+        // reason for this is that if an error occurs we don't actually know if
+        // the file descriptor was closed or not, and if we retried (for
+        // something like EINTR), we might close another valid file descriptor
+        // (opened after we closed ours.
+        let _ = unsafe { libc::close(self.fd) };
     }
 }

src/libstd/sys/unix/fs2.rs

@@ -205,19 +205,27 @@ impl OpenOptions {
 
 impl File {
     pub fn open(path: &Path, opts: &OpenOptions) -> io::Result<File> {
+        let path = try!(cstr(path));
+        File::open_c(&path, opts)
+    }
+
+    pub fn open_c(path: &CStr, opts: &OpenOptions) -> io::Result<File> {
         let flags = opts.flags | match (opts.read, opts.write) {
             (true, true) => libc::O_RDWR,
             (false, true) => libc::O_WRONLY,
             (true, false) |
             (false, false) => libc::O_RDONLY,
         };
-        let path = try!(cstr(path));
         let fd = try!(cvt_r(|| unsafe {
             libc::open(path.as_ptr(), flags, opts.mode)
         }));
-        Ok(File(FileDesc::new(fd)))
+        let fd = FileDesc::new(fd);
+        fd.set_cloexec();
+        Ok(File(fd))
     }
 
+    pub fn into_fd(self) -> FileDesc { self.0 }
+
     pub fn file_attr(&self) -> io::Result<FileAttr> {
         let mut stat: libc::stat = unsafe { mem::zeroed() };
         try!(cvt(unsafe { libc::fstat(self.0.raw(), &mut stat) }));

src/libstd/sys/unix/net.rs

@@ -47,7 +47,9 @@ impl Socket {
         };
         unsafe {
             let fd = try!(cvt(libc::socket(fam, ty, 0)));
-            Ok(Socket(FileDesc::new(fd)))
+            let fd = FileDesc::new(fd);
+            fd.set_cloexec();
+            Ok(Socket(fd))
         }
     }
 
@@ -56,13 +58,16 @@ impl Socket {
         let fd = try!(cvt_r(|| unsafe {
             libc::accept(self.0.raw(), storage, len)
         }));
-        Ok(Socket(FileDesc::new(fd)))
+        let fd = FileDesc::new(fd);
+        fd.set_cloexec();
+        Ok(Socket(fd))
     }
 
     pub fn duplicate(&self) -> io::Result<Socket> {
-        cvt(unsafe { libc::dup(self.0.raw()) }).map(|fd| {
-            Socket(FileDesc::new(fd))
-        })
+        let fd = try!(cvt(unsafe { libc::dup(self.0.raw()) }));
+        let fd = FileDesc::new(fd);
+        fd.set_cloexec();
+        Ok(Socket(fd))
     }
 
     pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> {

src/libstd/sys/unix/pipe2.rs

@@ -20,19 +20,20 @@ use libc;
 
 pub struct AnonPipe(FileDesc);
 
-pub unsafe fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
+pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
     let mut fds = [0; 2];
-    if libc::pipe(fds.as_mut_ptr()) == 0 {
-        Ok((AnonPipe::from_fd(fds[0]),
-            AnonPipe::from_fd(fds[1])))
+    if unsafe { libc::pipe(fds.as_mut_ptr()) == 0 } {
+        Ok((AnonPipe::from_fd(fds[0]), AnonPipe::from_fd(fds[1])))
     } else {
         Err(io::Error::last_os_error())
     }
 }
 
 impl AnonPipe {
     pub fn from_fd(fd: libc::c_int) -> AnonPipe {
-        AnonPipe(FileDesc::new(fd))
+        let fd = FileDesc::new(fd);
+        fd.set_cloexec();
+        AnonPipe(fd)
     }
 
     pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> {
@@ -43,7 +44,7 @@ impl AnonPipe {
         self.0.write(buf)
     }
 
-    pub fn raw(&self) -> libc::c_int {
-        self.0.raw()
+    pub fn into_fd(self) -> FileDesc {
+        self.0
     }
 }