Skip to content

Commit 55ca27f

Browse files
committed
use rwlock for accessing ENV
1 parent cfba499 commit 55ca27f

File tree

3 files changed

+55
-11
lines changed

3 files changed

+55
-11
lines changed

library/std/src/sys/unix/os.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use crate::str;
2222
use crate::sys::cvt;
2323
use crate::sys::fd;
2424
use crate::sys_common::mutex::{StaticMutex, StaticMutexGuard};
25+
use crate::sys_common::rwlock::{RWLock, RWLockGuard};
2526
use crate::vec;
2627

2728
use libc::{c_char, c_int, c_void};
@@ -493,17 +494,16 @@ pub unsafe fn environ() -> *mut *const *const c_char {
493494
&mut environ
494495
}
495496

496-
pub unsafe fn env_lock() -> StaticMutexGuard {
497-
// It is UB to attempt to acquire this mutex reentrantly!
498-
static ENV_LOCK: StaticMutex = StaticMutex::new();
499-
ENV_LOCK.lock()
497+
pub unsafe fn env_rwlock(readonly: bool) -> RWLockGuard {
498+
static ENV_LOCK: RWLock = RWLock::new();
499+
if readonly { ENV_LOCK.read_with_guard() } else { ENV_LOCK.write_with_guard() }
500500
}
501501

502502
/// Returns a vector of (variable, value) byte-vector pairs for all the
503503
/// environment variables of the current process.
504504
pub fn env() -> Env {
505505
unsafe {
506-
let _guard = env_lock();
506+
let _guard = env_rwlock(true);
507507
let mut environ = *environ();
508508
let mut result = Vec::new();
509509
if !environ.is_null() {
@@ -540,7 +540,7 @@ pub fn getenv(k: &OsStr) -> io::Result<Option<OsString>> {
540540
// always None as well
541541
let k = CString::new(k.as_bytes())?;
542542
unsafe {
543-
let _guard = env_lock();
543+
let _guard = env_rwlock(true);
544544
let s = libc::getenv(k.as_ptr()) as *const libc::c_char;
545545
let ret = if s.is_null() {
546546
None
@@ -556,7 +556,7 @@ pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
556556
let v = CString::new(v.as_bytes())?;
557557

558558
unsafe {
559-
let _guard = env_lock();
559+
let _guard = env_rwlock(false);
560560
cvt(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop)
561561
}
562562
}
@@ -565,7 +565,7 @@ pub fn unsetenv(n: &OsStr) -> io::Result<()> {
565565
let nbuf = CString::new(n.as_bytes())?;
566566

567567
unsafe {
568-
let _guard = env_lock();
568+
let _guard = env_rwlock(false);
569569
cvt(libc::unsetenv(nbuf.as_ptr())).map(drop)
570570
}
571571
}

library/std/src/sys/unix/process/process_unix.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ impl Command {
4747
// a lock any more because the parent won't do anything and the child is
4848
// in its own process.
4949
let result = unsafe {
50-
let _env_lock = sys::os::env_lock();
50+
let _env_lock = sys::os::env_rwlock(true);
5151
cvt(libc::fork())?
5252
};
5353

@@ -124,7 +124,7 @@ impl Command {
124124
// Similar to when forking, we want to ensure that access to
125125
// the environment is synchronized, so make sure to grab the
126126
// environment lock before we try to exec.
127-
let _lock = sys::os::env_lock();
127+
let _lock = sys::os::env_rwlock(true);
128128

129129
let Err(e) = self.do_exec(theirs, envp.as_ref());
130130
e
@@ -404,7 +404,7 @@ impl Command {
404404
cvt_nz(libc::posix_spawnattr_setflags(attrs.0.as_mut_ptr(), flags as _))?;
405405

406406
// Make sure we synchronize access to the global `environ` resource
407-
let _env_lock = sys::os::env_lock();
407+
let _env_lock = sys::os::env_rwlock(true);
408408
let envp = envp.map(|c| c.as_ptr()).unwrap_or_else(|| *sys::os::environ() as *const _);
409409
cvt_nz(libc::posix_spawnp(
410410
&mut p.pid,

library/std/src/sys_common/rwlock.rs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,23 @@
11
use crate::sys::rwlock as imp;
22

3+
enum GuardType {
4+
Read,
5+
Write,
6+
}
7+
8+
pub struct RWLockGuard(&'static RWLock, GuardType);
9+
10+
impl Drop for RWLockGuard {
11+
fn drop(&mut self) {
12+
unsafe {
13+
match &self.1 {
14+
GuardType::Read => self.0.read_unlock(),
15+
GuardType::Write => self.0.write_unlock(),
16+
}
17+
}
18+
}
19+
}
20+
321
/// An OS-based reader-writer lock.
422
///
523
/// This structure is entirely unsafe and serves as the lowest layer of a
@@ -26,6 +44,19 @@ impl RWLock {
2644
self.0.read()
2745
}
2846

47+
/// Acquires shared access to the underlying lock, blocking the current
48+
/// thread to do so.
49+
///
50+
/// The lock is automatically unlocked when the returned guard is dropped.
51+
///
52+
/// Behavior is undefined if the rwlock has been moved between this and any
53+
/// previous method call.
54+
#[inline]
55+
pub unsafe fn read_with_guard(&'static self) -> RWLockGuard {
56+
self.read();
57+
RWLockGuard(&self, GuardType::Read)
58+
}
59+
2960
/// Attempts to acquire shared access to this lock, returning whether it
3061
/// succeeded or not.
3162
///
@@ -48,6 +79,19 @@ impl RWLock {
4879
self.0.write()
4980
}
5081

82+
/// Acquires write access to the underlying lock, blocking the current thread
83+
/// to do so.
84+
///
85+
/// The lock is automatically unlocked when the returned guard is dropped.
86+
///
87+
/// Behavior is undefined if the rwlock has been moved between this and any
88+
/// previous method call.
89+
#[inline]
90+
pub unsafe fn write_with_guard(&'static self) -> RWLockGuard {
91+
self.write();
92+
RWLockGuard(&self, GuardType::Write)
93+
}
94+
5195
/// Attempts to acquire exclusive access to this lock, returning whether it
5296
/// succeeded or not.
5397
///

0 commit comments

Comments
 (0)