From 9a4d42a78a37593c6f8f18c7f393d31f571e82ab Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Tue, 4 Jun 2024 22:55:19 -0700 Subject: [PATCH] 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. 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. On x86_64 Linux, this change removes all libpthreads dependencies: ```diff - pthread_mutex_lock - pthread_mutex_unlock ``` and adds these dependencies: ```diff + std::panicking::panic_count::is_zero_slow_path + std::sys::sync::mutex::futex::Mutex::lock_contended + std::sys::sync::mutex::futex::Mutex::wake + std::panicking::panic_count::GLOBAL_PANIC_COUNT ``` 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] https://github.com/rust-random/getrandom/issues/453#issuecomment-2148124364 --- src/error.rs | 2 ++ src/use_file.rs | 30 ++++++------------------------ 2 files changed, 8 insertions(+), 24 deletions(-) diff --git a/src/error.rs b/src/error.rs index 5eff99eb..4317efd0 100644 --- a/src/error.rs +++ b/src/error.rs @@ -58,6 +58,8 @@ impl Error { pub const NODE_ES_MODULE: Error = internal_error(14); /// Calling Windows ProcessPrng failed. pub const WINDOWS_PROCESS_PRNG: Error = internal_error(15); + /// The mutex used when opening the random file was poisoned. + pub const UNEXPECTED_FILE_MUTEX_POISONED: Error = internal_error(15); /// Codes below this point represent OS Errors (i.e. positive i32 values). /// Codes at or above this point, but below [`Error::CUSTOM_START`] are diff --git a/src/use_file.rs b/src/use_file.rs index 36210dee..e42a744b 100644 --- a/src/use_file.rs +++ b/src/use_file.rs @@ -4,11 +4,12 @@ use crate::{ Error, }; use core::{ - cell::UnsafeCell, ffi::c_void, mem::MaybeUninit, sync::atomic::{AtomicUsize, Ordering::Relaxed}, }; +extern crate std; +use std::sync::{Mutex, PoisonError}; /// For all platforms, we use `/dev/urandom` rather than `/dev/random`. /// For more information see the linked man pages in lib.rs. @@ -44,11 +45,10 @@ fn get_rng_fd() -> Result { #[cold] fn get_fd_locked() -> Result { - // SAFETY: We use the mutex only in this method, and we always unlock it - // before returning, making sure we don't violate the pthread_mutex_t API. - static MUTEX: Mutex = Mutex::new(); - unsafe { MUTEX.lock() }; - let _guard = DropGuard(|| unsafe { MUTEX.unlock() }); + static MUTEX: Mutex<()> = Mutex::new(()); + let _guard = MUTEX + .lock() + .map_err(|_: PoisonError<_>| Error::UNEXPECTED_FILE_MUTEX_POISONED)?; if let Some(fd) = get_fd() { return Ok(fd); @@ -129,24 +129,6 @@ fn wait_until_rng_ready() -> Result<(), Error> { } } -struct Mutex(UnsafeCell); - -impl Mutex { - const fn new() -> Self { - Self(UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER)) - } - unsafe fn lock(&self) { - let r = libc::pthread_mutex_lock(self.0.get()); - debug_assert_eq!(r, 0); - } - unsafe fn unlock(&self) { - let r = libc::pthread_mutex_unlock(self.0.get()); - debug_assert_eq!(r, 0); - } -} - -unsafe impl Sync for Mutex {} - struct DropGuard(F); impl Drop for DropGuard {