diff --git a/src/lib.rs b/src/lib.rs index fe000c3..d19b9c7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -231,13 +231,6 @@ impl Client { /// result with the connected client will be returned. In other cases /// result will contain `Err(FromEnvErr)`. /// - /// Note that on Unix the `Client` returned **takes ownership of the file - /// descriptors specified in the environment**. Jobservers on Unix are - /// implemented with `pipe` file descriptors, and they're inherited from - /// parent processes. This `Client` returned takes ownership of the file - /// descriptors for this process and will close the file descriptors after - /// this value is dropped. - /// /// Additionally on Unix this function will configure the file descriptors /// with `CLOEXEC` so they're not automatically inherited by spawned /// children. @@ -256,11 +249,7 @@ impl Client { /// make sure to take ownership properly of the file descriptors passed /// down, if any. /// - /// It's generally unsafe to call this function twice in a program if the - /// previous invocation returned `Some`. - /// - /// Note, though, that on Windows it should be safe to call this function - /// any number of times. + /// It is ok to call this function any number of times. pub unsafe fn from_env_ext(check_pipe: bool) -> FromEnv { let (env, var_os) = match ["CARGO_MAKEFLAGS", "MAKEFLAGS", "MFLAGS"] .iter() @@ -293,6 +282,19 @@ impl Client { /// environment. /// /// Wraps `from_env_ext` and discards error details. + /// + /// # Safety + /// + /// This function is `unsafe` to call on Unix specifically as it + /// transitively requires usage of the `from_raw_fd` function, which is + /// itself unsafe in some circumstances. + /// + /// It's recommended to call this function very early in the lifetime of a + /// program before any other file descriptors are opened. That way you can + /// make sure to take ownership properly of the file descriptors passed + /// down, if any. + /// + /// It is ok to call this function any number of times. pub unsafe fn from_env() -> Option { Self::from_env_ext(false).client.ok() } diff --git a/src/unix.rs b/src/unix.rs index 12cbe05..25efef1 100644 --- a/src/unix.rs +++ b/src/unix.rs @@ -158,9 +158,10 @@ impl Client { } } - drop(set_cloexec(read, true)); - drop(set_cloexec(write, true)); - Ok(Some(Client::from_fds(read, write))) + Ok(Some(Client::Pipe { + read: clone_fd_and_set_cloexec(read)?, + write: clone_fd_and_set_cloexec(write)?, + })) } unsafe fn from_fds(read: c_int, write: c_int) -> Client { @@ -444,6 +445,14 @@ unsafe fn fd_check(fd: c_int, check_pipe: bool) -> Result<(), FromEnvErrorInner> } } +fn clone_fd_and_set_cloexec(fd: c_int) -> Result { + // Safety: fd is a valid fd dand it remains open until returns + unsafe { BorrowedFd::borrow_raw(fd) } + .try_clone_to_owned() + .map(File::from) + .map_err(|err| FromEnvErrorInner::CannotOpenFd(fd, err)) +} + fn set_cloexec(fd: c_int, set: bool) -> io::Result<()> { unsafe { let previous = cvt(libc::fcntl(fd, libc::F_GETFD))?;