Fix setting O_NONBLOCK in parallel::stderr::set_non_blocking (#946)

Author: NobodyXu

src/command_helpers.rs

@@ -167,7 +167,8 @@ impl StderrForwarder {
     pub(crate) fn set_non_blocking(&mut self) -> Result<(), Error> {
         assert!(!self.is_non_blocking);
 
-        if let Some((stderr, _)) = self.inner.as_mut() {
+        #[cfg(unix)]
+        if let Some((stderr, _)) = self.inner.as_ref() {
             crate::parallel::stderr::set_non_blocking(stderr)?;
         }
 

src/parallel/job_token/unix.rs

@@ -10,6 +10,8 @@ use std::{
     path::Path,
 };
 
+use crate::parallel::stderr::set_non_blocking;
+
 pub(super) struct JobServerClient {
     read: File,
     write: Option<File>,
@@ -35,7 +37,7 @@ impl JobServerClient {
         if is_pipe(&file)? {
             // File in Rust is always closed-on-exec as long as it's opened by
             // `File::open` or `fs::OpenOptions::open`.
-            set_nonblocking(&file)?;
+            set_non_blocking(&file).ok()?;
 
             Some(Self {
                 read: file,
@@ -79,8 +81,8 @@ impl JobServerClient {
                 let write = write.try_clone().ok()?;
 
                 // Set read and write end to nonblocking
-                set_nonblocking(&read)?;
-                set_nonblocking(&write)?;
+                set_non_blocking(&read).ok()?;
+                set_non_blocking(&write).ok()?;
 
                 Some(Self {
                     read,
@@ -135,21 +137,6 @@ impl JobServerClient {
     }
 }
 
-fn set_nonblocking(file: &File) -> Option<()> {
-    // F_SETFL can only set the O_APPEND, O_ASYNC, O_DIRECT, O_NOATIME, and
-    // O_NONBLOCK flags.
-    //
-    // For pipe, only O_NONBLOCK is meaningful, so it is ok to
-    // not issue a F_GETFL fcntl syscall.
-    let ret = unsafe { libc::fcntl(file.as_raw_fd(), libc::F_SETFL, libc::O_NONBLOCK) };
-
-    if ret == -1 {
-        None
-    } else {
-        Some(())
-    }
-}
-
 fn cvt(t: c_int) -> io::Result<c_int> {
     if t == -1 {
         Err(io::Error::last_os_error())

src/parallel/stderr.rs

@@ -6,29 +6,32 @@ use crate::{Error, ErrorKind};
 #[cfg(all(not(unix), not(windows)))]
 compile_error!("Only unix and windows support non-blocking pipes! For other OSes, disable the parallel feature.");
 
-#[allow(unused_variables)]
-pub fn set_non_blocking(stderr: &mut ChildStderr) -> Result<(), Error> {
+#[cfg(unix)]
+pub fn set_non_blocking(pipe: &impl std::os::unix::io::AsRawFd) -> Result<(), Error> {
     // On Unix, switch the pipe to non-blocking mode.
     // On Windows, we have a different way to be non-blocking.
-    #[cfg(unix)]
-    {
-        use std::os::unix::io::AsRawFd;
-        let fd = stderr.as_raw_fd();
-        debug_assert_eq!(
-            unsafe { libc::fcntl(fd, libc::F_GETFL, 0) },
-            0,
-            "stderr should have no flags set"
-        );
+    let fd = pipe.as_raw_fd();
+    let flags = unsafe { libc::fcntl(fd, libc::F_GETFL, 0) };
+    if flags == -1 {
+        return Err(Error::new(
+            ErrorKind::IOError,
+            format!(
+                "Failed to get flags for pipe {}: {}",
+                fd,
+                std::io::Error::last_os_error()
+            ),
+        ));
+    }
 
-        if unsafe { libc::fcntl(fd, libc::F_SETFL, libc::O_NONBLOCK) } != 0 {
-            return Err(Error::new(
-                ErrorKind::IOError,
-                format!(
-                    "Failed to set flags for child stderr: {}",
-                    std::io::Error::last_os_error()
-                ),
-            ));
-        }
+    if unsafe { libc::fcntl(fd, libc::F_SETFL, flags | libc::O_NONBLOCK) } == -1 {
+        return Err(Error::new(
+            ErrorKind::IOError,
+            format!(
+                "Failed to set flags for pipe {}: {}",
+                fd,
+                std::io::Error::last_os_error()
+            ),
+        ));
     }
 
     Ok(())