Optimize: Rm pre_exec call on Command (#75)

* Rm `imp::Client::pre_run`

On Unix `Client` always clone the fds and now it also records the original fd
used to construct the `Client` and pass it to children, there's no need
to unset `CLOEXEC` before exec anymore.

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

* Fix test: Add back `pre_run` for owned pipe

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

* Fix

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

* Fix

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

---------

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

src/lib.rs

@@ -504,8 +504,7 @@ impl Client {
         Cmd: Command,
         F: FnOnce(&mut Cmd) -> io::Result<R>,
     {
-        // Register one-time callback on unix to unset CLO_EXEC
-        // in child process.
+        #[cfg(unix)]
         self.0.inner.pre_run(&mut cmd);
 
         let arg = self.0.inner.string_arg();

src/unix.rs

@@ -29,7 +29,7 @@ pub struct Client {
     write: File,
     creation_arg: ClientCreationArg,
     /// If the Client owns the fifo, then we should remove it on drop.
-    owns_fifo: bool,
+    owns: bool,
 }
 
 #[derive(Debug)]
@@ -42,7 +42,8 @@ impl Client {
         // Create nonblocking and cloexec pipes
         let pipes = create_pipe()?;
 
-        let client = unsafe { Self::from_fds(pipes[0], pipes[1]) };
+        let mut client = unsafe { Self::from_fds(pipes[0], pipes[1]) };
+        client.owns = true;
 
         client.init(limit)?;
 
@@ -84,7 +85,7 @@ impl Client {
                         read: file.try_clone()?,
                         write: file,
                         creation_arg: ClientCreationArg::Fifo(name.into_boxed_path()),
-                        owns_fifo: true,
+                        owns: true,
                     };
 
                     client.init(limit)?;
@@ -143,7 +144,7 @@ impl Client {
                 read,
                 write: open_file_rw(path).ok()?,
                 creation_arg: ClientCreationArg::Fifo(path.into()),
-                owns_fifo: false,
+                owns: false,
             })
         } else {
             None
@@ -200,7 +201,7 @@ impl Client {
                         read,
                         write,
                         creation_arg,
-                        owns_fifo: false,
+                        owns: false,
                     });
                 }
 
@@ -211,7 +212,7 @@ impl Client {
                     read,
                     write,
                     creation_arg,
-                    owns_fifo: false,
+                    owns: false,
                 })
             }
             _ => None,
@@ -223,7 +224,7 @@ impl Client {
             read: File::from_raw_fd(read),
             write: File::from_raw_fd(write),
             creation_arg: ClientCreationArg::Fds { read, write },
-            owns_fifo: false,
+            owns: false,
         }
     }
 
@@ -301,6 +302,10 @@ impl Client {
     where
         Cmd: Command,
     {
+        if !self.owns || self.get_fifo().is_some() {
+            return;
+        }
+
         let read = self.read.as_raw_fd();
         let write = self.write.as_raw_fd();
 
@@ -344,7 +349,7 @@ impl Client {
 impl Drop for Client {
     fn drop(&mut self) {
         if let Some(path) = self.get_fifo() {
-            if self.owns_fifo {
+            if self.owns {
                 fs::remove_file(path).ok();
             }
         }

src/wasm.rs

@@ -98,13 +98,6 @@ impl Client {
         );
     }
 
-    pub fn pre_run<Cmd>(&self, _cmd: &mut Cmd) {
-        panic!(
-            "On this platform there is no cross process jobserver support,
-             so Client::configure_and_run is not supported."
-        );
-    }
-
     pub fn available(&self) -> io::Result<usize> {
         Ok(*self.count())
     }

src/windows.rs

@@ -176,14 +176,6 @@ impl Client {
         Cow::Borrowed(&self.name)
     }
 
-    pub fn pre_run<Cmd>(&self, _cmd: &mut Cmd)
-    where
-        Cmd: Command,
-    {
-        // nothing to do here, we gave the name of our semaphore to the
-        // child above
-    }
-
     pub fn available(&self) -> io::Result<usize> {
         // Can't read value of a semaphore on Windows, so
         // try to acquire without sleeping, since we can find out the