Do not attempt to unlock envlock in child process after a fork.
This is a breaking change for cases where the environment is accessed in a Command::pre_exec closure. Except for single-threaded programs these uses were not correct anyway since they aren't async-signal safe.
This commit is contained in:
parent
3a5d45f68c
commit
d854789ce1
@ -62,9 +62,14 @@ pub trait CommandExt: Sealed {
|
||||
/// `fork`. This primarily means that any modifications made to memory on
|
||||
/// behalf of this closure will **not** be visible to the parent process.
|
||||
/// This is often a very constrained environment where normal operations
|
||||
/// like `malloc` or acquiring a mutex are not guaranteed to work (due to
|
||||
/// like `malloc`, accessing environment variables through [`std::env`]
|
||||
/// or acquiring a mutex are not guaranteed to work (due to
|
||||
/// other threads perhaps still running when the `fork` was run).
|
||||
///
|
||||
/// For further details refer to the [POSIX fork() specification]
|
||||
/// and the equivalent documentation for any targeted
|
||||
/// platform, especially the requirements around *async-signal-safety*.
|
||||
///
|
||||
/// This also means that all resources such as file descriptors and
|
||||
/// memory-mapped regions got duplicated. It is your responsibility to make
|
||||
/// sure that the closure does not violate library invariants by making
|
||||
@ -73,6 +78,10 @@ pub trait CommandExt: Sealed {
|
||||
/// When this closure is run, aspects such as the stdio file descriptors and
|
||||
/// working directory have successfully been changed, so output to these
|
||||
/// locations may not appear where intended.
|
||||
///
|
||||
/// [POSIX fork() specification]:
|
||||
/// https://pubs.opengroup.org/onlinepubs/9699919799/functions/fork.html
|
||||
/// [`std::env`]: mod@crate::env
|
||||
#[stable(feature = "process_pre_exec", since = "1.34.0")]
|
||||
unsafe fn pre_exec<F>(&mut self, f: F) -> &mut process::Command
|
||||
where
|
||||
|
@ -1,6 +1,7 @@
|
||||
use crate::convert::TryInto;
|
||||
use crate::fmt;
|
||||
use crate::io::{self, Error, ErrorKind};
|
||||
use crate::mem;
|
||||
use crate::ptr;
|
||||
use crate::sys;
|
||||
use crate::sys::cvt;
|
||||
@ -45,15 +46,14 @@ impl Command {
|
||||
//
|
||||
// Note that as soon as we're done with the fork there's no need to hold
|
||||
// a lock any more because the parent won't do anything and the child is
|
||||
// in its own process.
|
||||
let result = unsafe {
|
||||
let _env_lock = sys::os::env_lock();
|
||||
cvt(libc::fork())?
|
||||
};
|
||||
// in its own process. Thus the parent drops the lock guard while the child
|
||||
// forgets it to avoid unlocking it on a new thread, which would be invalid.
|
||||
let (env_lock, result) = unsafe { (sys::os::env_lock(), cvt(libc::fork())?) };
|
||||
|
||||
let pid = unsafe {
|
||||
match result {
|
||||
0 => {
|
||||
mem::forget(env_lock);
|
||||
drop(input);
|
||||
let Err(err) = self.do_exec(theirs, envp.as_ref());
|
||||
let errno = err.raw_os_error().unwrap_or(libc::EINVAL) as u32;
|
||||
@ -74,7 +74,10 @@ impl Command {
|
||||
rtassert!(output.write(&bytes).is_ok());
|
||||
libc::_exit(1)
|
||||
}
|
||||
n => n,
|
||||
n => {
|
||||
drop(env_lock);
|
||||
n
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
|
@ -43,20 +43,6 @@ fn main() {
|
||||
assert!(output.stderr.is_empty());
|
||||
assert_eq!(output.stdout, b"hello\nhello2\n");
|
||||
|
||||
let output = unsafe {
|
||||
Command::new(&me)
|
||||
.arg("test2")
|
||||
.pre_exec(|| {
|
||||
env::set_var("FOO", "BAR");
|
||||
Ok(())
|
||||
})
|
||||
.output()
|
||||
.unwrap()
|
||||
};
|
||||
assert!(output.status.success());
|
||||
assert!(output.stderr.is_empty());
|
||||
assert!(output.stdout.is_empty());
|
||||
|
||||
let output = unsafe {
|
||||
Command::new(&me)
|
||||
.arg("test3")
|
||||
|
Loading…
Reference in New Issue
Block a user