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

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.

2 files changed, 25 insertions, 24 deletions

diff --git a/src/libstd/sys/windows/c.rs b/src/libstd/sys/windows/c.rs
index b168081ff8a..270e260d504 100644
--- a/src/libstd/sys/windows/c.rs
+++ b/src/libstd/sys/windows/c.rs
@@ -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;
diff --git a/src/libstd/sys/windows/os.rs b/src/libstd/sys/windows/os.rs
index 1680ea88d0b..ba4bce38014 100644
--- a/src/libstd/sys/windows/os.rs
+++ b/src/libstd/sys/windows/os.rs
@@ -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();