Skip to content

Commit b02470b

Browse files
committed
use_file: std::sync::Mutex, dropping all libpthread use.
pthreads mutexes are not safe to move. While it is very unlikely that the mutex we create will ever be moved, we don't actively do anything to actively prevent it from being moved. (libstd, when it used/uses pthreads mutexes, would box them to prevent them from being moved.) Also, now on Linux and Android (and many other targets for which we don't use use_std), libstd uses futexes instead of pthreads mutexes. Thus using libstd's Mutex will be more efficient and avoid adding an often-otherwise-unnecessary libpthreads dependency on these targets. * Linux, Android: Futex [1]. * Haiku, Redox, NTO, AIX: pthreads [2]. * others: not using `use_file`. This will not affect our plans for *-*-linux-none, since we don't plan to use `use_file` for it. OnceLock This breaks 32-bit x86 QNX Neutrino, which doesn't have libstd because the target itself is abandoned [3]. the other QNX Neutrino targets didn't get libstd support until Rust 1.69, so this effectively raises the MSRV for them to 1.69. Otherwise, the MSRV increases to 1.63 for the above-mentioned targets, as that's when `Mutex::new()` became a `const fn`. I tried to use `Once` to avoid the MSRV increase but it doesn't support fallible initialization even in Nightly. `OnceLock` wasn't added until 1.70. On x86_64 Linux, this change removes all libpthreads dependencies: ```diff - pthread_mutex_lock - pthread_mutex_unlock ``` and adds these libstd dependencies: ```diff + std::panicking::panic_count::GLOBAL_PANIC_COUNT + std::panicking::panic_count::is_zero_slow_path + std::sys::sync::mutex::futex::Mutex::lock_contended + std::sys::sync::mutex::futex::Mutex::wake ``` as measured using `cargo asm`. [1] https://github.com/rust-lang/rust/blob/c1dba09f263cbff6170f130aa418e28bdf22bd96/library/std/src/sys/sync/mutex/mod.rs#L4-L10 [2] https://github.com/rust-lang/rust/blob/c1dba09f263cbff6170f130aa418e28bdf22bd96/library/std/src/sys/sync/mutex/mod.rs#L17-L20 [3] rust-random#453 (comment)
1 parent 7ac1473 commit b02470b

File tree

2 files changed

+14
-30
lines changed

2 files changed

+14
-30
lines changed

src/error.rs

+3
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ impl Error {
5858
pub const NODE_ES_MODULE: Error = internal_error(14);
5959
/// Calling Windows ProcessPrng failed.
6060
pub const WINDOWS_PROCESS_PRNG: Error = internal_error(15);
61+
/// The mutex used when opening the random file was poisoned.
62+
pub const UNEXPECTED_FILE_MUTEX_POISONED: Error = internal_error(16);
6163

6264
/// Codes below this point represent OS Errors (i.e. positive i32 values).
6365
/// Codes at or above this point, but below [`Error::CUSTOM_START`] are
@@ -175,6 +177,7 @@ fn internal_desc(error: Error) -> Option<&'static str> {
175177
Error::NODE_RANDOM_FILL_SYNC => Some("Calling Node.js API crypto.randomFillSync failed"),
176178
Error::NODE_ES_MODULE => Some("Node.js ES modules are not directly supported, see https://docs.rs/getrandom#nodejs-es-module-support"),
177179
Error::WINDOWS_PROCESS_PRNG => Some("ProcessPrng: Windows system function failure"),
180+
Error::UNEXPECTED_FILE_MUTEX_POISONED => Some("File: Initialization panicked, poisoning the mutex"),
178181
_ => None,
179182
}
180183
}

src/use_file.rs

+11-30
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ extern crate std;
44

55
use crate::{util_libc::sys_fill_exact, Error};
66
use core::{
7-
cell::UnsafeCell,
87
ffi::c_void,
98
mem::MaybeUninit,
109
sync::atomic::{AtomicI32, Ordering},
@@ -14,6 +13,7 @@ use std::{
1413
io,
1514
// TODO(MSRV 1.66): use `std::os::fd` instead of `std::unix::io`.
1615
os::unix::io::{AsRawFd as _, BorrowedFd, IntoRawFd as _, RawFd},
16+
sync::{Mutex, PoisonError},
1717
};
1818

1919
/// For all platforms, we use `/dev/urandom` rather than `/dev/random`.
@@ -70,9 +70,16 @@ fn get_rng_fd() -> Result<BorrowedFd<'static>, Error> {
7070
// descriptors concurrently, which could run into the limit on the
7171
// number of open file descriptors. Our goal is to have no more than one
7272
// file descriptor open, ever.
73-
static MUTEX: Mutex = Mutex::new();
74-
unsafe { MUTEX.lock() };
75-
let _guard = DropGuard(|| unsafe { MUTEX.unlock() });
73+
//
74+
// We assume any call to `Mutex::lock` synchronizes-with
75+
// (Ordering::Acquire) the preceding dropping of a `MutexGuard` that
76+
// unlocks the mutex (Ordering::Release) and that `Mutex` doesn't have
77+
// any special treatment for what's "inside" the mutex (the `T` in
78+
// `Mutex<T>`). See `https://github.com/rust-lang/rust/issues/126239.
79+
static MUTEX: Mutex<()> = Mutex::new(());
80+
let _guard = MUTEX
81+
.lock()
82+
.map_err(|_: PoisonError<_>| Error::UNEXPECTED_FILE_MUTEX_POISONED)?;
7683

7784
if let Some(fd) = get_fd() {
7885
return Ok(fd);
@@ -168,29 +175,3 @@ fn map_io_error(err: io::Error) -> Error {
168175
}
169176
})
170177
}
171-
172-
struct Mutex(UnsafeCell<libc::pthread_mutex_t>);
173-
174-
impl Mutex {
175-
const fn new() -> Self {
176-
Self(UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER))
177-
}
178-
unsafe fn lock(&self) {
179-
let r = libc::pthread_mutex_lock(self.0.get());
180-
debug_assert_eq!(r, 0);
181-
}
182-
unsafe fn unlock(&self) {
183-
let r = libc::pthread_mutex_unlock(self.0.get());
184-
debug_assert_eq!(r, 0);
185-
}
186-
}
187-
188-
unsafe impl Sync for Mutex {}
189-
190-
struct DropGuard<F: FnMut()>(F);
191-
192-
impl<F: FnMut()> Drop for DropGuard<F> {
193-
fn drop(&mut self) {
194-
self.0()
195-
}
196-
}

0 commit comments

Comments
 (0)