-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Description
I'm pretty new at rust so please forgive any bad terminology etc.
Here's my test case:
use std::env;
use std::process::Command;
use std::os::unix::process::CommandExt;
fn main() {
let key = "VAR";
env::set_var(key, "val1");
match env::var(key) {
Ok(val) => println!("{}: {:?}", key, val),
Err(e) => println!("couldn't interpret {}: {}", key, e),
}
Command::new("does-not-exist").env(key, "val2").exec();
match env::var(key) {
Ok(val) => println!("{}: {:?}", key, val),
Err(e) => println!("couldn't interpret {}: {}", key, e),
}
}
Running it (on Ubuntu 18.04 fwiw) I get:
mwhudson@ringil:~/tmp$ ./t
VAR: "val1"
couldn't interpret VAR: environment variable not found
What happened? Well https://github.com/rust-lang/rust/blob/1.29.2/src/libstd/sys/unix/process/process_unix.rs#L197 happened:
pub fn exec(&mut self, default: Stdio) -> io::Error {
let envp = self.capture_env();
...
if let Some(envp) = maybe_envp {
*sys::os::environ() = envp.as_ptr();
}
This overwrites the environ pointer on the assumption that the following call to exec succeeds and makes any effect on other code in process irrelevant, but if it fails nothing restores the "right" value.
I found this because a cargo test that calls cargo run with a bogus runner configured segfaulted on exit sometimes, and this turned out to be because a shared library linked to cargo (gnutls) calls getenv in it's finalizer. Getting from there to the above took some serious time in gdb :)
For a fix, the easiest thing to be would be to call execvpe instead of execvp. But that's a GNU extension. Another option would be to call execve and do the PATH searching in library code. Or put the old environ pointer back if execvp returns but that seems kinda gross somehow.