Fix nonblocking setting in parallel::job_token

Refactor it to use `parallel::stderr::set_non_blocking`, so that there's
only one implementation and it would fix the bug.

Signed-off-by: Jiahao XU <[email protected]>

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() {
+        if let Some((stderr, _)) = self.inner.as_ref() {
+            #[cfg(unix)]
             crate::parallel::stderr::set_non_blocking(stderr)?;
         }
 

src/lib.rs

@@ -1499,6 +1499,7 @@ impl Build {
                 };
                 let mut child = spawn(&mut cmd, &program, &self.cargo_output)?;
                 let mut stderr_forwarder = StderrForwarder::new(&mut child);
+                #[cfg(unix)]
                 stderr_forwarder.set_non_blocking()?;
 
                 cell_update(&pendings, |mut pendings| {

src/parallel/job_token/unix.rs

@@ -13,6 +13,8 @@ use std::{
     path::Path,
 };
 
+use crate::parallel::stderr::set_non_blocking;
+
 pub(super) struct JobServerClient {
     read: File,
     write: Option<File>,
@@ -48,7 +50,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,
@@ -92,8 +94,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)?;
+                set_non_blocking(&write)?;
 
                 Some(Self {
                     read,
@@ -148,21 +150,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,20 +6,19 @@ 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();
+        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 child stderr: {}",
+                    "Failed to get flags for pipe {}: {}",
+                    fd,
                     std::io::Error::last_os_error()
                 ),
             ));
@@ -29,7 +28,8 @@ pub fn set_non_blocking(stderr: &mut ChildStderr) -> Result<(), Error> {
             return Err(Error::new(
                 ErrorKind::IOError,
                 format!(
-                    "Failed to set flags for child stderr: {}",
+                    "Failed to set flags for pipe {}: {}",
+                    fd,
                     std::io::Error::last_os_error()
                 ),
             ));