Auto merge of rust-lang#31417 - alexcrichton:cloexec-all-the-things, …

…r=brson

These commits finish up closing out rust-lang#24237 by filling out all locations we create new file descriptors with variants that atomically create the file descriptor and set CLOEXEC where possible. Previous support for doing this in `File::open` was added in rust-lang#27971 and support for `try_clone` was added in rust-lang#27980. This commit fills out:

* `Socket::new` now passes `SOCK_CLOEXEC`
* `Socket::accept` now uses `accept4`
* `pipe2` is used instead of `pipe`

Unfortunately most of this support is Linux-specific, and most of it is post-2.6.18 (our oldest supported version), so all of the detection here is done dynamically. It looks like OSX does not have equivalent variants for these functions, so there's nothing more we can do there. Support for BSDs can be added over time if they also have these functions.

Closes rust-lang#24237

src/libstd/sys/unix/fd.rs

@@ -77,9 +77,7 @@ impl FileDesc {
         // follow a strategy similar to musl [1] where if passing
         // F_DUPFD_CLOEXEC causes `fcntl` to return EINVAL it means it's not
         // supported (the third parameter, 0, is always valid), so we stop
-        // trying that. We also *still* call the `set_cloexec` method as
-        // apparently some kernel at some point stopped setting CLOEXEC even
-        // though it reported doing so on F_DUPFD_CLOEXEC.
+        // trying that.
         //
         // Also note that Android doesn't have F_DUPFD_CLOEXEC, but get it to
         // resolve so we at least compile this.
@@ -95,14 +93,25 @@ impl FileDesc {
             fd.set_cloexec();
             fd
         };
-        static TRY_CLOEXEC: AtomicBool = AtomicBool::new(true);
+        static TRY_CLOEXEC: AtomicBool =
+            AtomicBool::new(!cfg!(target_os = "android"));
         let fd = self.raw();
-        if !cfg!(target_os = "android") && TRY_CLOEXEC.load(Ordering::Relaxed) {
+        if TRY_CLOEXEC.load(Ordering::Relaxed) {
             match cvt(unsafe { libc::fcntl(fd, F_DUPFD_CLOEXEC, 0) }) {
+                // We *still* call the `set_cloexec` method as apparently some
+                // linux kernel at some point stopped setting CLOEXEC even
+                // though it reported doing so on F_DUPFD_CLOEXEC.
+                Ok(fd) => {
+                    return Ok(if cfg!(target_os = "linux") {
+                        make_filedesc(fd)
+                    } else {
+                        FileDesc::new(fd)
+                    })
+                }
                 Err(ref e) if e.raw_os_error() == Some(libc::EINVAL) => {
                     TRY_CLOEXEC.store(false, Ordering::Relaxed);
                 }
-                res => return res.map(make_filedesc),
+                Err(e) => return Err(e),
             }
         }
         cvt(unsafe { libc::fcntl(fd, libc::F_DUPFD, 0) }).map(make_filedesc)

src/libstd/sys/unix/fs.rs

@@ -413,10 +413,18 @@ impl File {
             libc::open(path.as_ptr(), flags, opts.mode as c_int)
         }));
         let fd = FileDesc::new(fd);
-        // Even though we open with the O_CLOEXEC flag, still set CLOEXEC here,
-        // in case the open flag is not supported (it's just ignored by the OS
-        // in that case).
-        fd.set_cloexec();
+
+        // Currently the standard library supports Linux 2.6.18 which did not
+        // have the O_CLOEXEC flag (passed above). If we're running on an older
+        // Linux kernel then the flag is just ignored by the OS, so we continue
+        // to explicitly ask for a CLOEXEC fd here.
+        //
+        // The CLOEXEC flag, however, is supported on versions of OSX/BSD/etc
+        // that we support, so we only do this on Linux currently.
+        if cfg!(target_os = "linux") {
+            fd.set_cloexec();
+        }
+
         Ok(File(fd))
     }
 

src/libstd/sys/unix/mod.rs

@@ -27,6 +27,9 @@ use ops::Neg;
 #[cfg(target_os = "openbsd")]   pub use os::openbsd as platform;
 #[cfg(target_os = "solaris")]   pub use os::solaris as platform;
 
+#[macro_use]
+pub mod weak;
+
 pub mod backtrace;
 pub mod condvar;
 pub mod ext;

src/libstd/sys/unix/net.rs

@@ -12,7 +12,7 @@ use prelude::v1::*;
 
 use ffi::CStr;
 use io;
-use libc::{self, c_int, size_t};
+use libc::{self, c_int, size_t, sockaddr, socklen_t};
 use net::{SocketAddr, Shutdown};
 use str;
 use sys::fd::FileDesc;
@@ -25,6 +25,16 @@ pub use libc as netc;
 
 pub type wrlen_t = size_t;
 
+// See below for the usage of SOCK_CLOEXEC, but this constant is only defined on
+// Linux currently (e.g. support doesn't exist on other platforms). In order to
+// get name resolution to work and things to compile we just define a dummy
+// SOCK_CLOEXEC here for other platforms. Note that the dummy constant isn't
+// actually ever used (the blocks below are wrapped in `if cfg!` as well.
+#[cfg(target_os = "linux")]
+use libc::SOCK_CLOEXEC;
+#[cfg(not(target_os = "linux"))]
+const SOCK_CLOEXEC: c_int = 0;
+
 pub struct Socket(FileDesc);
 
 pub fn init() {}
@@ -48,15 +58,48 @@ impl Socket {
             SocketAddr::V6(..) => libc::AF_INET6,
         };
         unsafe {
+            // On linux we first attempt to pass the SOCK_CLOEXEC flag to
+            // atomically create the socket and set it as CLOEXEC. Support for
+            // this option, however, was added in 2.6.27, and we still support
+            // 2.6.18 as a kernel, so if the returned error is EINVAL we
+            // fallthrough to the fallback.
+            if cfg!(target_os = "linux") {
+                match cvt(libc::socket(fam, ty | SOCK_CLOEXEC, 0)) {
+                    Ok(fd) => return Ok(Socket(FileDesc::new(fd))),
+                    Err(ref e) if e.raw_os_error() == Some(libc::EINVAL) => {}
+                    Err(e) => return Err(e),
+                }
+            }
+
             let fd = try!(cvt(libc::socket(fam, ty, 0)));
             let fd = FileDesc::new(fd);
             fd.set_cloexec();
             Ok(Socket(fd))
         }
     }
 
-    pub fn accept(&self, storage: *mut libc::sockaddr,
-                  len: *mut libc::socklen_t) -> io::Result<Socket> {
+    pub fn accept(&self, storage: *mut sockaddr, len: *mut socklen_t)
+                  -> io::Result<Socket> {
+        // Unfortunately the only known way right now to accept a socket and
+        // atomically set the CLOEXEC flag is to use the `accept4` syscall on
+        // Linux. This was added in 2.6.28, however, and because we support
+        // 2.6.18 we must detect this support dynamically.
+        if cfg!(target_os = "linux") {
+            weak! {
+                fn accept4(c_int, *mut sockaddr, *mut socklen_t, c_int) -> c_int
+            }
+            if let Some(accept) = accept4.get() {
+                let res = cvt_r(|| unsafe {
+                    accept(self.0.raw(), storage, len, SOCK_CLOEXEC)
+                });
+                match res {
+                    Ok(fd) => return Ok(Socket(FileDesc::new(fd))),
+                    Err(ref e) if e.raw_os_error() == Some(libc::ENOSYS) => {}
+                    Err(e) => return Err(e),
+                }
+            }
+        }
+
         let fd = try!(cvt_r(|| unsafe {
             libc::accept(self.0.raw(), storage, len)
         }));

src/libstd/sys/unix/pipe.rs

@@ -8,9 +8,10 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use sys::fd::FileDesc;
 use io;
-use libc;
+use libc::{self, c_int};
+use sys::cvt_r;
+use sys::fd::FileDesc;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Anonymous pipes
@@ -20,6 +21,24 @@ pub struct AnonPipe(FileDesc);
 
 pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
     let mut fds = [0; 2];
+
+    // Unfortunately the only known way right now to create atomically set the
+    // CLOEXEC flag is to use the `pipe2` syscall on Linux. This was added in
+    // 2.6.27, however, and because we support 2.6.18 we must detect this
+    // support dynamically.
+    if cfg!(target_os = "linux") {
+        weak! { fn pipe2(*mut c_int, c_int) -> c_int }
+        if let Some(pipe) = pipe2.get() {
+            match cvt_r(|| unsafe { pipe(fds.as_mut_ptr(), libc::O_CLOEXEC) }) {
+                Ok(_) => {
+                    return Ok((AnonPipe(FileDesc::new(fds[0])),
+                               AnonPipe(FileDesc::new(fds[1]))))
+                }
+                Err(ref e) if e.raw_os_error() == Some(libc::ENOSYS) => {}
+                Err(e) => return Err(e),
+            }
+        }
+    }
     if unsafe { libc::pipe(fds.as_mut_ptr()) == 0 } {
         Ok((AnonPipe::from_fd(fds[0]), AnonPipe::from_fd(fds[1])))
     } else {

src/libstd/sys/unix/thread.rs

@@ -317,37 +317,12 @@ pub mod guard {
 // storage.  We need that information to avoid blowing up when a small stack
 // is created in an application with big thread-local storage requirements.
 // See #6233 for rationale and details.
-//
-// Use dlsym to get the symbol value at runtime, both for
-// compatibility with older versions of glibc, and to avoid creating
-// dependencies on GLIBC_PRIVATE symbols.  Assumes that we've been
-// dynamically linked to libpthread but that is currently always the
-// case.  We previously used weak linkage (under the same assumption),
-// but that caused Debian to detect an unnecessarily strict versioned
-// dependency on libc6 (#23628).
 #[cfg(target_os = "linux")]
 #[allow(deprecated)]
 fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize {
-    use dynamic_lib::DynamicLibrary;
-    use sync::Once;
-
-    type F = unsafe extern "C" fn(*const libc::pthread_attr_t) -> libc::size_t;
-    static INIT: Once = Once::new();
-    static mut __pthread_get_minstack: Option<F> = None;
-
-    INIT.call_once(|| {
-        let lib = match DynamicLibrary::open(None) {
-            Ok(l) => l,
-            Err(..) => return,
-        };
-        unsafe {
-            if let Ok(f) = lib.symbol("__pthread_get_minstack") {
-                __pthread_get_minstack = Some(mem::transmute::<*const (), F>(f));
-            }
-        }
-    });
+    weak!(fn __pthread_get_minstack(*const libc::pthread_attr_t) -> libc::size_t);
 
-    match unsafe { __pthread_get_minstack } {
+    match __pthread_get_minstack.get() {
         None => libc::PTHREAD_STACK_MIN as usize,
         Some(f) => unsafe { f(attr) as usize },
     }

src/libstd/sys/unix/weak.rs

@@ -0,0 +1,85 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+//! Support for "weak linkage" to symbols on Unix
+//!
+//! Some I/O operations we do in libstd require newer versions of OSes but we
+//! need to maintain binary compatibility with older releases for now. In order
+//! to use the new functionality when available we use this module for
+//! detection.
+//!
+//! One option to use here is weak linkage, but that is unfortunately only
+//! really workable on Linux. Hence, use dlsym to get the symbol value at
+//! runtime. This is also done for compatibility with older versions of glibc,
+//! and to avoid creating dependencies on GLIBC_PRIVATE symbols. It assumes that
+//! we've been dynamically linked to the library the symbol comes from, but that
+//! is currently always the case for things like libpthread/libc.
+//!
+//! A long time ago this used weak linkage for the __pthread_get_minstack
+//! symbol, but that caused Debian to detect an unnecessarily strict versioned
+//! dependency on libc6 (#23628).
+
+use libc;
+
+use ffi::CString;
+use marker;
+use mem;
+use sync::atomic::{AtomicUsize, Ordering};
+
+macro_rules! weak {
+    (fn $name:ident($($t:ty),*) -> $ret:ty) => (
+        static $name: ::sys::weak::Weak<unsafe extern fn($($t),*) -> $ret> =
+            ::sys::weak::Weak::new(stringify!($name));
+    )
+}
+
+pub struct Weak<F> {
+    name: &'static str,
+    addr: AtomicUsize,
+    _marker: marker::PhantomData<F>,
+}
+
+impl<F> Weak<F> {
+    pub const fn new(name: &'static str) -> Weak<F> {
+        Weak {
+            name: name,
+            addr: AtomicUsize::new(1),
+            _marker: marker::PhantomData,
+        }
+    }
+
+    pub fn get(&self) -> Option<&F> {
+        assert_eq!(mem::size_of::<F>(), mem::size_of::<usize>());
+        unsafe {
+            if self.addr.load(Ordering::SeqCst) == 1 {
+                self.addr.store(fetch(self.name), Ordering::SeqCst);
+            }
+            if self.addr.load(Ordering::SeqCst) == 0 {
+                None
+            } else {
+                mem::transmute::<&AtomicUsize, Option<&F>>(&self.addr)
+            }
+        }
+    }
+}
+
+unsafe fn fetch(name: &str) -> usize {
+    let name = match CString::new(name) {
+        Ok(cstr) => cstr,
+        Err(..) => return 0,
+    };
+    let lib = libc::dlopen(0 as *const _, libc::RTLD_LAZY);
+    if lib.is_null() {
+        return 0
+    }
+    let ret = libc::dlsym(lib, name.as_ptr()) as usize;
+    libc::dlclose(lib);
+    return ret
+}

src/test/run-pass/fds-are-cloexec.rs

@@ -16,11 +16,11 @@
 extern crate libc;
 
 use std::env;
-use std::fs::{self, File};
+use std::fs::File;
 use std::io;
 use std::net::{TcpListener, TcpStream, UdpSocket};
 use std::os::unix::prelude::*;
-use std::process::Command;
+use std::process::{Command, Stdio};
 use std::thread;
 
 fn main() {
@@ -45,6 +45,17 @@ fn parent() {
     let udp1 = UdpSocket::bind("127.0.0.1:0").unwrap();
     let udp2 = udp1.try_clone().unwrap();
 
+    let mut child = Command::new(env::args().next().unwrap())
+                            .arg("100")
+                            .stdout(Stdio::piped())
+                            .stdin(Stdio::piped())
+                            .stderr(Stdio::piped())
+                            .spawn().unwrap();
+    let pipe1 = child.stdin.take().unwrap();
+    let pipe2 = child.stdout.take().unwrap();
+    let pipe3 = child.stderr.take().unwrap();
+
+
     let status = Command::new(env::args().next().unwrap())
                         .arg(file.as_raw_fd().to_string())
                         .arg(tcp1.as_raw_fd().to_string())
@@ -55,9 +66,13 @@ fn parent() {
                         .arg(tcp6.as_raw_fd().to_string())
                         .arg(udp1.as_raw_fd().to_string())
                         .arg(udp2.as_raw_fd().to_string())
+                        .arg(pipe1.as_raw_fd().to_string())
+                        .arg(pipe2.as_raw_fd().to_string())
+                        .arg(pipe3.as_raw_fd().to_string())
                         .status()
                         .unwrap();
     assert!(status.success());
+    child.wait().unwrap();
 }
 
 fn child(args: &[String]) {