From e22143c075c7aee0a3a5bc90b51adff6cd250b34 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sun, 14 Mar 2021 19:10:34 +0100 Subject: [PATCH] Revert "Revert "use RWlock when accessing os::env #81850"" This reverts commit acdca316c3d42299d31c1b47eb792006ffdfc29c. --- library/std/src/sys/unix/os.rs | 19 +++--- .../std/src/sys/unix/process/process_unix.rs | 6 +- library/std/src/sys_common/rwlock.rs | 59 +++++++++++++++++++ 3 files changed, 72 insertions(+), 12 deletions(-) diff --git a/library/std/src/sys/unix/os.rs b/library/std/src/sys/unix/os.rs index d5e14bec76572..1d1118aa69434 100644 --- a/library/std/src/sys/unix/os.rs +++ b/library/std/src/sys/unix/os.rs @@ -22,6 +22,7 @@ use crate::str; use crate::sys::cvt; use crate::sys::fd; use crate::sys_common::mutex::{StaticMutex, StaticMutexGuard}; +use crate::sys_common::rwlock::{RWLockReadGuard, StaticRWLock}; use crate::vec; use libc::{c_char, c_int, c_void}; @@ -490,20 +491,20 @@ pub unsafe fn environ() -> *mut *const *const c_char { extern "C" { static mut environ: *const *const c_char; } - &mut environ + ptr::addr_of_mut!(environ) } -pub unsafe fn env_lock() -> StaticMutexGuard { - // It is UB to attempt to acquire this mutex reentrantly! - static ENV_LOCK: StaticMutex = StaticMutex::new(); - ENV_LOCK.lock() +static ENV_LOCK: StaticRWLock = StaticRWLock::new(); + +pub fn env_read_lock() -> RWLockReadGuard { + ENV_LOCK.read_with_guard() } /// Returns a vector of (variable, value) byte-vector pairs for all the /// environment variables of the current process. pub fn env() -> Env { unsafe { - let _guard = env_lock(); + let _guard = env_read_lock(); let mut environ = *environ(); let mut result = Vec::new(); if !environ.is_null() { @@ -540,7 +541,7 @@ pub fn getenv(k: &OsStr) -> io::Result> { // always None as well let k = CString::new(k.as_bytes())?; unsafe { - let _guard = env_lock(); + let _guard = env_read_lock(); let s = libc::getenv(k.as_ptr()) as *const libc::c_char; let ret = if s.is_null() { None @@ -556,7 +557,7 @@ pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { let v = CString::new(v.as_bytes())?; unsafe { - let _guard = env_lock(); + let _guard = ENV_LOCK.write_with_guard(); cvt(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop) } } @@ -565,7 +566,7 @@ pub fn unsetenv(n: &OsStr) -> io::Result<()> { let nbuf = CString::new(n.as_bytes())?; unsafe { - let _guard = env_lock(); + let _guard = ENV_LOCK.write_with_guard(); cvt(libc::unsetenv(nbuf.as_ptr())).map(drop) } } diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 2eb64a99e599a..47aaca82af946 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -48,7 +48,7 @@ impl Command { // a lock any more because the parent won't do anything and the child is // 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 (env_lock, result) = unsafe { (sys::os::env_read_lock(), cvt(libc::fork())?) }; let pid = unsafe { match result { @@ -127,7 +127,7 @@ impl Command { // Similar to when forking, we want to ensure that access to // the environment is synchronized, so make sure to grab the // environment lock before we try to exec. - let _lock = sys::os::env_lock(); + let _lock = sys::os::env_read_lock(); let Err(e) = self.do_exec(theirs, envp.as_ref()); e @@ -407,7 +407,7 @@ impl Command { cvt_nz(libc::posix_spawnattr_setflags(attrs.0.as_mut_ptr(), flags as _))?; // Make sure we synchronize access to the global `environ` resource - let _env_lock = sys::os::env_lock(); + let _env_lock = sys::os::env_read_lock(); let envp = envp.map(|c| c.as_ptr()).unwrap_or_else(|| *sys::os::environ() as *const _); cvt_nz(libc::posix_spawnp( &mut p.pid, diff --git a/library/std/src/sys_common/rwlock.rs b/library/std/src/sys_common/rwlock.rs index 3705d641a1be6..70b31b19f824c 100644 --- a/library/std/src/sys_common/rwlock.rs +++ b/library/std/src/sys_common/rwlock.rs @@ -86,3 +86,62 @@ impl RWLock { self.0.destroy() } } + +// the cfg annotations only exist due to dead code warnings. the code itself is portable +#[cfg(unix)] +pub struct StaticRWLock(RWLock); + +#[cfg(unix)] +impl StaticRWLock { + pub const fn new() -> StaticRWLock { + StaticRWLock(RWLock::new()) + } + + /// Acquires shared access to the underlying lock, blocking the current + /// thread to do so. + /// + /// The lock is automatically unlocked when the returned guard is dropped. + #[inline] + pub fn read_with_guard(&'static self) -> RWLockReadGuard { + // SAFETY: All methods require static references, therefore self + // cannot be moved between invocations. + unsafe { + self.0.read(); + } + RWLockReadGuard(&self.0) + } + + /// Acquires write access to the underlying lock, blocking the current thread + /// to do so. + /// + /// The lock is automatically unlocked when the returned guard is dropped. + #[inline] + pub fn write_with_guard(&'static self) -> RWLockWriteGuard { + // SAFETY: All methods require static references, therefore self + // cannot be moved between invocations. + unsafe { + self.0.write(); + } + RWLockWriteGuard(&self.0) + } +} + +#[cfg(unix)] +pub struct RWLockReadGuard(&'static RWLock); + +#[cfg(unix)] +impl Drop for RWLockReadGuard { + fn drop(&mut self) { + unsafe { self.0.read_unlock() } + } +} + +#[cfg(unix)] +pub struct RWLockWriteGuard(&'static RWLock); + +#[cfg(unix)] +impl Drop for RWLockWriteGuard { + fn drop(&mut self) { + unsafe { self.0.write_unlock() } + } +}