OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #29305 - alexcrichton:bad-getenv, r=brson

Browse Source 
As discovered in #29298, `env::set_var("", "")` will panic, but it turns out
that it *also* deadlocks on Unix systems. This happens because if a panic
happens while holding the environment lock, we then go try to read
RUST_BACKTRACE, grabbing the environment lock, causing a deadlock.

Specifically, the changes made here are:

* The environment lock is pushed into `std::sys` instead of `std::env`. This
  also only puts it in the Unix implementation, not Windows where the functions
  are already threadsafe.
* The `std::sys` implementation now returns `io::Result` so panics are
  explicitly at the `std::env` level.
This commit is contained in:
bors 2015-11-06 00:56:08 +00:00
commit 41308a0520
4 changed files with 62 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
src/libstd/env.rs
View File

@ -23,7 +23,6 @@ use ffi::{OsStr, OsString};
use fmt;
use io;
use path::{Path, PathBuf};
use sync::StaticMutex;
use sys::os as os_imp;
/// Returns the current working directory as a `PathBuf`.
@ -68,8 +67,6 @@ pub fn set_current_dir<P: AsRef<Path>>(p: P) -> io::Result<()> {
os_imp::chdir(p.as_ref())
}
static ENV_LOCK: StaticMutex = StaticMutex::new();
/// An iterator over a snapshot of the environment variables of this process.
///
/// This iterator is created through `std::env::vars()` and yields `(String,
@ -133,7 +130,6 @@ pub fn vars() -> Vars {
/// ```
#[stable(feature = "env", since = "1.0.0")]
pub fn vars_os() -> VarsOs {
let _g = ENV_LOCK.lock();
VarsOs { inner: os_imp::env() }
}
@ -204,8 +200,9 @@ pub fn var_os<K: AsRef<OsStr>>(key: K) -> Option<OsString> {
}
fn _var_os(key: &OsStr) -> Option<OsString> {
let _g = ENV_LOCK.lock();
os_imp::getenv(key)
os_imp::getenv(key).unwrap_or_else(|e| {
panic!("failed to get environment variable `{:?}`: {}", key, e)
})
}
/// Possible errors from the `env::var` method.
@ -281,8 +278,10 @@ pub fn set_var<K: AsRef<OsStr>, V: AsRef<OsStr>>(k: K, v: V) {
}
fn _set_var(k: &OsStr, v: &OsStr) {
let _g = ENV_LOCK.lock();
os_imp::setenv(k, v)
os_imp::setenv(k, v).unwrap_or_else(|e| {
panic!("failed to set environment variable `{:?}` to `{:?}`: {}",
k, v, e)
})
}
/// Removes an environment variable from the environment of the currently running process.
@ -322,8 +321,9 @@ pub fn remove_var<K: AsRef<OsStr>>(k: K) {
}
fn _remove_var(k: &OsStr) {
let _g = ENV_LOCK.lock();
os_imp::unsetenv(k)
os_imp::unsetenv(k).unwrap_or_else(|e| {
panic!("failed to remove environment variable `{:?}`: {}", k, e)
})
}
/// An iterator over `Path` instances for parsing an environment variable

49
src/libstd/sys/unix/os.rs
View File

@ -26,11 +26,14 @@ use path::{self, PathBuf};
use ptr;
use slice;
use str;
use sync::StaticMutex;
use sys::c;
use sys::fd;
use sys::cvt;
use vec;
const TMPBUF_SZ: usize = 128;
static ENV_LOCK: StaticMutex = StaticMutex::new();
/// Returns the platform-specific value of errno
pub fn errno() -> i32 {
@ -378,6 +381,7 @@ pub unsafe fn environ() -> *mut *const *const c_char {
/// Returns a vector of (variable, value) byte-vector pairs for all the
/// environment variables of the current process.
pub fn env() -> Env {
let _g = ENV_LOCK.lock();
return unsafe {
let mut environ = *environ();
if environ as usize == 0 {
@ -401,35 +405,36 @@ pub fn env() -> Env {
}
}
pub fn getenv(k: &OsStr) -> Option<OsString> {
unsafe {
let s = k.to_cstring().unwrap();
let s = libc::getenv(s.as_ptr()) as *const _;
pub fn getenv(k: &OsStr) -> io::Result<Option<OsString>> {
// environment variables with a nul byte can't be set, so their value is
// always None as well
let k = try!(CString::new(k.as_bytes()));
let _g = ENV_LOCK.lock();
Ok(unsafe {
let s = libc::getenv(k.as_ptr()) as *const _;
if s.is_null() {
None
} else {
Some(OsStringExt::from_vec(CStr::from_ptr(s).to_bytes().to_vec()))
}
}
})
}
pub fn setenv(k: &OsStr, v: &OsStr) {
unsafe {
let k = k.to_cstring().unwrap();
let v = v.to_cstring().unwrap();
if libc::funcs::posix01::unistd::setenv(k.as_ptr(), v.as_ptr(), 1) != 0 {
panic!("failed setenv: {}", io::Error::last_os_error());
}
}
pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
let k = try!(CString::new(k.as_bytes()));
let v = try!(CString::new(v.as_bytes()));
let _g = ENV_LOCK.lock();
cvt(unsafe {
libc::funcs::posix01::unistd::setenv(k.as_ptr(), v.as_ptr(), 1)
}).map(|_| ())
}
pub fn unsetenv(n: &OsStr) {
unsafe {
let nbuf = n.to_cstring().unwrap();
if libc::funcs::posix01::unistd::unsetenv(nbuf.as_ptr()) != 0 {
panic!("failed unsetenv: {}", io::Error::last_os_error());
}
}
pub fn unsetenv(n: &OsStr) -> io::Result<()> {
let nbuf = try!(CString::new(n.as_bytes()));
let _g = ENV_LOCK.lock();
cvt(unsafe {
libc::funcs::posix01::unistd::unsetenv(nbuf.as_ptr())
}).map(|_| ())
}
pub fn page_size() -> usize {
@ -439,7 +444,7 @@ pub fn page_size() -> usize {
}
pub fn temp_dir() -> PathBuf {
getenv("TMPDIR".as_ref()).map(PathBuf::from).unwrap_or_else(|| {
::env::var_os("TMPDIR").map(PathBuf::from).unwrap_or_else(|| {
if cfg!(target_os = "android") {
PathBuf::from("/data/local/tmp")
} else {
@ -449,7 +454,7 @@ pub fn temp_dir() -> PathBuf {
}
pub fn home_dir() -> Option<PathBuf> {
return getenv("HOME".as_ref()).or_else(|| unsafe {
return ::env::var_os("HOME").or_else(|| unsafe {
fallback()
}).map(PathBuf::from);

1
src/libstd/sys/windows/c.rs
View File

@ -32,6 +32,7 @@ pub const WSASYS_STATUS_LEN: usize = 128;
pub const FIONBIO: libc::c_long = 0x8004667e;
pub const FD_SETSIZE: usize = 64;
pub const MSG_DONTWAIT: libc::c_int = 0;
pub const ERROR_ENVVAR_NOT_FOUND: libc::c_int = 203;
pub const ERROR_ILLEGAL_CHARACTER: libc::c_int = 582;
pub const ENABLE_ECHO_INPUT: libc::DWORD = 0x4;
pub const ENABLE_EXTENDED_FLAGS: libc::DWORD = 0x80;

48
src/libstd/sys/windows/os.rs
View File

@ -26,7 +26,7 @@ use os::windows::ffi::EncodeWide;
use path::{self, PathBuf};
use ptr;
use slice;
use sys::c;
use sys::{c, cvt};
use sys::handle::Handle;
use libc::funcs::extra::kernel32::{
@ -248,41 +248,41 @@ pub fn chdir(p: &path::Path) -> io::Result<()> {
let mut p = p.encode_wide().collect::<Vec<_>>();
p.push(0);
unsafe {
match libc::SetCurrentDirectoryW(p.as_ptr()) != (0 as libc::BOOL) {
true => Ok(()),
false => Err(io::Error::last_os_error()),
}
}
cvt(unsafe {
libc::SetCurrentDirectoryW(p.as_ptr())
}).map(|_| ())
}
pub fn getenv(k: &OsStr) -> Option<OsString> {
pub fn getenv(k: &OsStr) -> io::Result<Option<OsString>> {
let k = super::to_utf16_os(k);
super::fill_utf16_buf(|buf, sz| unsafe {
let res = super::fill_utf16_buf(|buf, sz| unsafe {
libc::GetEnvironmentVariableW(k.as_ptr(), buf, sz)
}, |buf| {
OsStringExt::from_wide(buf)
}).ok()
});
match res {
Ok(value) => Ok(Some(value)),
Err(ref e) if e.raw_os_error() == Some(c::ERROR_ENVVAR_NOT_FOUND) => {
Ok(None)
}
Err(e) => Err(e)
}
}
pub fn setenv(k: &OsStr, v: &OsStr) {
pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
let k = super::to_utf16_os(k);
let v = super::to_utf16_os(v);
unsafe {
if libc::SetEnvironmentVariableW(k.as_ptr(), v.as_ptr()) == 0 {
panic!("failed to set env: {}", io::Error::last_os_error());
}
}
cvt(unsafe {
libc::SetEnvironmentVariableW(k.as_ptr(), v.as_ptr())
}).map(|_| ())
}
pub fn unsetenv(n: &OsStr) {
pub fn unsetenv(n: &OsStr) -> io::Result<()> {
let v = super::to_utf16_os(n);
unsafe {
if libc::SetEnvironmentVariableW(v.as_ptr(), ptr::null()) == 0 {
panic!("failed to unset env: {}", io::Error::last_os_error());
}
}
cvt(unsafe {
libc::SetEnvironmentVariableW(v.as_ptr(), ptr::null())
}).map(|_| ())
}
pub struct Args {
@ -339,8 +339,8 @@ pub fn temp_dir() -> PathBuf {
}
pub fn home_dir() -> Option<PathBuf> {
getenv("HOME".as_ref()).or_else(|| {
getenv("USERPROFILE".as_ref())
::env::var_os("HOME").or_else(|| {
::env::var_os("USERPROFILE")
}).map(PathBuf::from).or_else(|| unsafe {
let me = c::GetCurrentProcess();
let mut token = ptr::null_mut();