Skip to content

Commit

Permalink
use_file: std::sync::Mutex, dropping all libpthread use.
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
briansmith committed Jun 5, 2024
1 parent f5aaab8 commit 9fdb890
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 36 deletions.
2 changes: 1 addition & 1 deletion .clippy.toml
Original file line number Diff line number Diff line change
@@ -1 +1 @@
msrv = "1.57"
msrv = "1.63"
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-22.04, windows-2022]
toolchain: [nightly, beta, stable, 1.57]
toolchain: [nightly, beta, stable, 1.63]
# Only Test macOS on stable to reduce macOS CI jobs
include:
# x86_64-apple-darwin.
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
name = "getrandom"
version = "0.2.15" # Also update html_root_url in lib.rs when bumping this
edition = "2021"
rust-version = "1.57" # Sync .clippy.toml, tests.yml, and README.md.
rust-version = "1.63" # Sync .clippy.toml, tests.yml, and README.md.
authors = ["The Rand Project Developers"]
license = "MIT OR Apache-2.0"
description = "A small cross-platform library for retrieving random data from system source"
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ crate features, WASM support and Custom RNGs see the

## Minimum Supported Rust Version

This crate requires Rust 1.57.0 or later.
This crate requires Rust 1.63.0 or later.

## Platform Support

Expand Down
2 changes: 2 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 5 additions & 32 deletions src/use_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ extern crate std;

use crate::{util_libc::sys_fill_exact, Error};
use core::{
cell::UnsafeCell,
ffi::c_void,
mem::MaybeUninit,
sync::atomic::{AtomicI32, Ordering::Relaxed},
};
use std::{
fs, io,
os::fd::{IntoRawFd as _, RawFd},
sync::{Mutex, PoisonError},
};

/// For all platforms, we use `/dev/urandom` rather than `/dev/random`.
Expand Down Expand Up @@ -47,11 +47,10 @@ fn get_rng_fd() -> Result<RawFd, Error> {

#[cold]
fn get_fd_locked() -> Result<RawFd, Error> {
// 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);
Expand Down Expand Up @@ -151,29 +150,3 @@ fn map_io_error(err: io::Error) -> Error {
}
})
}

struct Mutex(UnsafeCell<libc::pthread_mutex_t>);

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: FnMut()>(F);

impl<F: FnMut()> Drop for DropGuard<F> {
fn drop(&mut self) {
self.0()
}
}

0 comments on commit 9fdb890

Please sign in to comment.