Skip to content

Commit

Permalink
std: use sync::Mutex for internal statics
Browse files Browse the repository at this point in the history
  • Loading branch information
joboet committed Oct 13, 2022
1 parent fa0ca78 commit 2d2c9e4
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 123 deletions.
6 changes: 2 additions & 4 deletions library/std/src/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,7 @@ impl Backtrace {
// Capture a backtrace which start just before the function addressed by
// `ip`
fn create(ip: usize) -> Backtrace {
// SAFETY: We don't attempt to lock this reentrantly.
let _lock = unsafe { lock() };
let _lock = lock();
let mut frames = Vec::new();
let mut actual_start = None;
unsafe {
Expand Down Expand Up @@ -469,8 +468,7 @@ impl Capture {
// Use the global backtrace lock to synchronize this as it's a
// requirement of the `backtrace` crate, and then actually resolve
// everything.
// SAFETY: We don't attempt to lock this reentrantly.
let _lock = unsafe { lock() };
let _lock = lock();
for frame in self.frames.iter_mut() {
let symbols = &mut frame.symbols;
let frame = match &frame.frame {
Expand Down
74 changes: 25 additions & 49 deletions library/std/src/sys/hermit/args.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,37 @@
use crate::ffi::OsString;
use crate::ffi::{c_char, CStr, OsString};
use crate::fmt;
use crate::os::unix::ffi::OsStringExt;
use crate::ptr;
use crate::sync::atomic::{
AtomicIsize, AtomicPtr,
Ordering::{Acquire, Relaxed, Release},
};
use crate::vec;

static ARGC: AtomicIsize = AtomicIsize::new(0);
static ARGV: AtomicPtr<*const u8> = AtomicPtr::new(ptr::null_mut());

/// One-time global initialization.
pub unsafe fn init(argc: isize, argv: *const *const u8) {
imp::init(argc, argv)
}

/// One-time global cleanup.
pub unsafe fn cleanup() {
imp::cleanup()
ARGC.store(argc, Relaxed);
// Use release ordering here to broadcast writes by the OS.
ARGV.store(argv as *mut *const u8, Release);
}

/// Returns the command line arguments
pub fn args() -> Args {
imp::args()
// Synchronize with the store above.
let argv = ARGV.load(Acquire);
// If argv has not been initialized yet, do not return any arguments.
let argc = if argv.is_null() { 0 } else { ARGC.load(Relaxed) };
let args: Vec<OsString> = (0..argc)
.map(|i| unsafe {
let cstr = CStr::from_ptr(*argv.offset(i) as *const c_char);
OsStringExt::from_vec(cstr.to_bytes().to_vec())
})
.collect();

Args { iter: args.into_iter() }
}

pub struct Args {
Expand Down Expand Up @@ -51,44 +68,3 @@ impl DoubleEndedIterator for Args {
self.iter.next_back()
}
}

mod imp {
use super::Args;
use crate::ffi::{CStr, OsString};
use crate::os::unix::ffi::OsStringExt;
use crate::ptr;

use crate::sys_common::mutex::StaticMutex;

static mut ARGC: isize = 0;
static mut ARGV: *const *const u8 = ptr::null();
static LOCK: StaticMutex = StaticMutex::new();

pub unsafe fn init(argc: isize, argv: *const *const u8) {
let _guard = LOCK.lock();
ARGC = argc;
ARGV = argv;
}

pub unsafe fn cleanup() {
let _guard = LOCK.lock();
ARGC = 0;
ARGV = ptr::null();
}

pub fn args() -> Args {
Args { iter: clone().into_iter() }
}

fn clone() -> Vec<OsString> {
unsafe {
let _guard = LOCK.lock();
(0..ARGC)
.map(|i| {
let cstr = CStr::from_ptr(*ARGV.offset(i) as *const i8);
OsStringExt::from_vec(cstr.to_bytes().to_vec())
})
.collect()
}
}
}
4 changes: 1 addition & 3 deletions library/std/src/sys/hermit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,7 @@ pub unsafe fn init(argc: isize, argv: *const *const u8, _sigpipe: u8) {

// SAFETY: must be called only once during runtime cleanup.
// NOTE: this is not guaranteed to run, for example when the program aborts.
pub unsafe fn cleanup() {
args::cleanup();
}
pub unsafe fn cleanup() {}

#[cfg(not(test))]
#[no_mangle]
Expand Down
6 changes: 3 additions & 3 deletions library/std/src/sys/windows/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::os::windows::ffi::{OsStrExt, OsStringExt};
use crate::os::windows::io::{AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle};
use crate::path::{Path, PathBuf};
use crate::ptr;
use crate::sync::Mutex;
use crate::sys::args::{self, Arg};
use crate::sys::c;
use crate::sys::c::NonZeroDWORD;
Expand All @@ -25,7 +26,6 @@ use crate::sys::handle::Handle;
use crate::sys::path;
use crate::sys::pipe::{self, AnonPipe};
use crate::sys::stdio;
use crate::sys_common::mutex::StaticMutex;
use crate::sys_common::process::{CommandEnv, CommandEnvs};
use crate::sys_common::IntoInner;

Expand Down Expand Up @@ -301,9 +301,9 @@ impl Command {
//
// For more information, msdn also has an article about this race:
// https://support.microsoft.com/kb/315939
static CREATE_PROCESS_LOCK: StaticMutex = StaticMutex::new();
static CREATE_PROCESS_LOCK: Mutex<()> = Mutex::new(());

let _guard = unsafe { CREATE_PROCESS_LOCK.lock() };
let _guard = CREATE_PROCESS_LOCK.lock();

let mut pipes = StdioPipes { stdin: None, stdout: None, stderr: None };
let null = Stdio::Null;
Expand Down
9 changes: 4 additions & 5 deletions library/std/src/sys_common/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@ use crate::fmt;
use crate::io;
use crate::io::prelude::*;
use crate::path::{self, Path, PathBuf};
use crate::sys_common::mutex::StaticMutex;
use crate::sync::{Mutex, PoisonError};

/// Max number of frames to print.
const MAX_NB_FRAMES: usize = 100;

// SAFETY: Don't attempt to lock this reentrantly.
pub unsafe fn lock() -> impl Drop {
static LOCK: StaticMutex = StaticMutex::new();
LOCK.lock()
pub fn lock() -> impl Drop {
static LOCK: Mutex<()> = Mutex::new(());
LOCK.lock().unwrap_or_else(PoisonError::into_inner)
}

/// Prints the current backtrace.
Expand Down
44 changes: 0 additions & 44 deletions library/std/src/sys_common/mutex.rs
Original file line number Diff line number Diff line change
@@ -1,49 +1,5 @@
use crate::sys::locks as imp;

/// An OS-based mutual exclusion lock, meant for use in static variables.
///
/// This mutex has a const constructor ([`StaticMutex::new`]), does not
/// implement `Drop` to cleanup resources, and causes UB when used reentrantly.
///
/// This mutex does not implement poisoning.
///
/// This is a wrapper around `imp::Mutex` that does *not* call `init()` and
/// `destroy()`.
pub struct StaticMutex(imp::Mutex);

unsafe impl Sync for StaticMutex {}

impl StaticMutex {
/// Creates a new mutex for use.
#[inline]
pub const fn new() -> Self {
Self(imp::Mutex::new())
}

/// Calls raw_lock() and then returns an RAII guard to guarantee the mutex
/// will be unlocked.
///
/// It is undefined behaviour to call this function while locked by the
/// same thread.
#[inline]
pub unsafe fn lock(&'static self) -> StaticMutexGuard {
self.0.lock();
StaticMutexGuard(&self.0)
}
}

#[must_use]
pub struct StaticMutexGuard(&'static imp::Mutex);

impl Drop for StaticMutexGuard {
#[inline]
fn drop(&mut self) {
unsafe {
self.0.unlock();
}
}
}

/// An OS-based mutual exclusion lock.
///
/// This mutex cleans up its resources in its `Drop` implementation, may safely
Expand Down
27 changes: 12 additions & 15 deletions library/std/src/thread/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1118,24 +1118,21 @@ impl ThreadId {
}
}
} else {
use crate::sys_common::mutex::StaticMutex;
use crate::sync::{Mutex, PoisonError};

// It is UB to attempt to acquire this mutex reentrantly!
static GUARD: StaticMutex = StaticMutex::new();
static mut COUNTER: u64 = 0;
static COUNTER: Mutex<u64> = Mutex::new(0);

unsafe {
let guard = GUARD.lock();
let mut counter = COUNTER.lock().unwrap_or_else(PoisonError::into_inner);
let Some(id) = counter.checked_add(1) else {
// in case the panic handler ends up calling `ThreadId::new()`,
// avoid reentrant lock acquire.
drop(counter);
exhausted();
};

let Some(id) = COUNTER.checked_add(1) else {
drop(guard); // in case the panic handler ends up calling `ThreadId::new()`, avoid reentrant lock acquire.
exhausted();
};

COUNTER = id;
drop(guard);
ThreadId(NonZeroU64::new(id).unwrap())
}
*counter = id;
drop(counter);
ThreadId(NonZeroU64::new(id).unwrap())
}
}
}
Expand Down

0 comments on commit 2d2c9e4

Please sign in to comment.