Skip to content

Commit acdca31

Browse files
committedMar 7, 2021
Revert "use RWlock when accessing os::env #81850"
This reverts commit 354f19c, reversing changes made to 0cfba2f.
1 parent 66ec64c commit acdca31

File tree

3 files changed

+12
-72
lines changed

3 files changed

+12
-72
lines changed
 

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

+9-10
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ 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::{RWLockReadGuard, StaticRWLock};
2625
use crate::vec;
2726

2827
use libc::{c_char, c_int, c_void};
@@ -491,20 +490,20 @@ pub unsafe fn environ() -> *mut *const *const c_char {
491490
extern "C" {
492491
static mut environ: *const *const c_char;
493492
}
494-
ptr::addr_of_mut!(environ)
493+
&mut environ
495494
}
496495

497-
static ENV_LOCK: StaticRWLock = StaticRWLock::new();
498-
499-
pub fn env_read_lock() -> RWLockReadGuard {
500-
ENV_LOCK.read_with_guard()
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()
501500
}
502501

503502
/// Returns a vector of (variable, value) byte-vector pairs for all the
504503
/// environment variables of the current process.
505504
pub fn env() -> Env {
506505
unsafe {
507-
let _guard = env_read_lock();
506+
let _guard = env_lock();
508507
let mut environ = *environ();
509508
let mut result = Vec::new();
510509
if !environ.is_null() {
@@ -541,7 +540,7 @@ pub fn getenv(k: &OsStr) -> io::Result<Option<OsString>> {
541540
// always None as well
542541
let k = CString::new(k.as_bytes())?;
543542
unsafe {
544-
let _guard = env_read_lock();
543+
let _guard = env_lock();
545544
let s = libc::getenv(k.as_ptr()) as *const libc::c_char;
546545
let ret = if s.is_null() {
547546
None
@@ -557,7 +556,7 @@ pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
557556
let v = CString::new(v.as_bytes())?;
558557

559558
unsafe {
560-
let _guard = ENV_LOCK.write_with_guard();
559+
let _guard = env_lock();
561560
cvt(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop)
562561
}
563562
}
@@ -566,7 +565,7 @@ pub fn unsetenv(n: &OsStr) -> io::Result<()> {
566565
let nbuf = CString::new(n.as_bytes())?;
567566

568567
unsafe {
569-
let _guard = ENV_LOCK.write_with_guard();
568+
let _guard = env_lock();
570569
cvt(libc::unsetenv(nbuf.as_ptr())).map(drop)
571570
}
572571
}

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

+3-3
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_read_lock();
50+
let _env_lock = sys::os::env_lock();
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_read_lock();
127+
let _lock = sys::os::env_lock();
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_read_lock();
407+
let _env_lock = sys::os::env_lock();
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

-59
Original file line numberDiff line numberDiff line change
@@ -86,62 +86,3 @@ impl RWLock {
8686
self.0.destroy()
8787
}
8888
}
89-
90-
// the cfg annotations only exist due to dead code warnings. the code itself is portable
91-
#[cfg(unix)]
92-
pub struct StaticRWLock(RWLock);
93-
94-
#[cfg(unix)]
95-
impl StaticRWLock {
96-
pub const fn new() -> StaticRWLock {
97-
StaticRWLock(RWLock::new())
98-
}
99-
100-
/// Acquires shared access to the underlying lock, blocking the current
101-
/// thread to do so.
102-
///
103-
/// The lock is automatically unlocked when the returned guard is dropped.
104-
#[inline]
105-
pub fn read_with_guard(&'static self) -> RWLockReadGuard {
106-
// SAFETY: All methods require static references, therefore self
107-
// cannot be moved between invocations.
108-
unsafe {
109-
self.0.read();
110-
}
111-
RWLockReadGuard(&self.0)
112-
}
113-
114-
/// Acquires write access to the underlying lock, blocking the current thread
115-
/// to do so.
116-
///
117-
/// The lock is automatically unlocked when the returned guard is dropped.
118-
#[inline]
119-
pub fn write_with_guard(&'static self) -> RWLockWriteGuard {
120-
// SAFETY: All methods require static references, therefore self
121-
// cannot be moved between invocations.
122-
unsafe {
123-
self.0.write();
124-
}
125-
RWLockWriteGuard(&self.0)
126-
}
127-
}
128-
129-
#[cfg(unix)]
130-
pub struct RWLockReadGuard(&'static RWLock);
131-
132-
#[cfg(unix)]
133-
impl Drop for RWLockReadGuard {
134-
fn drop(&mut self) {
135-
unsafe { self.0.read_unlock() }
136-
}
137-
}
138-
139-
#[cfg(unix)]
140-
pub struct RWLockWriteGuard(&'static RWLock);
141-
142-
#[cfg(unix)]
143-
impl Drop for RWLockWriteGuard {
144-
fn drop(&mut self) {
145-
unsafe { self.0.write_unlock() }
146-
}
147-
}

0 commit comments

Comments
 (0)