Skip to content

Commit

Permalink
Android/Linux/rdrand: Use once_cell::race::OnceBool instead of LazyBool.
Browse files Browse the repository at this point in the history
Remove src/lazy.rs.

`lazy::LazyBool` had "last to win the race" semantics. When multiple
threads see an uninitialized `LazyBool`, all of them will calculate a
value. As they finish, each one will overwrite the value set by the
thread that finished previously. If two threads calculate different
values for the boolean, then the value of the boolean can change
during the period where the threads are racing. This doesn't seem to be
a huge issue with the way it is currently used, but it is hard to
reason about.

`once_cell::race::OnceBool` has "first to win the race" semantics. When
multiple threads see an uninitialized `OnceBool`, all of them will
calculate a value. The first one to finish will write its value; the
rest will have their work ignored. Thus there is never any change in
the stored value at any point. This is much easier to reason about.

The different semantics come down to the fact that once_cell uses
`AtomicUsize::compare_exchange` whereas lazy.rs was using
`AtomicUsize::store`.
  • Loading branch information
briansmith committed Jun 19, 2024
1 parent 267639e commit 6aaba2f
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 76 deletions.
10 changes: 9 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ exclude = [".*"]

[dependencies]
cfg-if = "1"
once_cell = { version = "1.19.0", default-features = false, optional = true }

[target.'cfg(any(target_os = "android", target_os = "linux", all(target_arch = "x86_64", target_env = "sgx")))'.dependencies]
once_cell = { version = "1.19.0", default-features = false, features = ["race"] }

# When built as part of libstd
compiler_builtins = { version = "0.1", optional = true }
Expand All @@ -30,6 +34,10 @@ windows-targets = "0.52"
[target.'cfg(all(any(target_arch = "wasm32", target_arch = "wasm64"), target_os = "unknown"))'.dependencies]
wasm-bindgen = { version = "0.2.62", default-features = false, optional = true }
js-sys = { version = "0.3", optional = true }

[target.'cfg(any(target_arch = "x86", target_arch = "x86_64"))'.dev-dependencies]
once_cell = { version = "1.19.0", default-features = false, features = ["race"] }

[target.'cfg(all(any(target_arch = "wasm32", target_arch = "wasm64"), target_os = "unknown"))'.dev-dependencies]
wasm-bindgen-test = "0.3.18"

Expand All @@ -40,7 +48,7 @@ std = []
# Bumps minimum supported Linux kernel version to 3.17 and Android API level to 23 (Marshmallow).
linux_disable_fallback = []
# Feature to enable fallback RDRAND-based implementation on x86/x86_64
rdrand = []
rdrand = ["once_cell/race"]
# Feature to enable JavaScript bindings on wasm*-unknown-unknown
js = ["wasm-bindgen", "js-sys"]
# Feature to enable custom RNG implementations
Expand Down
66 changes: 0 additions & 66 deletions src/lazy.rs

This file was deleted.

1 change: 0 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ use crate::util::{slice_as_uninit_mut, slice_assume_init_mut};
use core::mem::MaybeUninit;

mod error;
mod lazy;
mod util;
// To prevent a breaking change when targets are added, we always export the
// register_custom_getrandom macro, so old Custom RNG crates continue to build.
Expand Down
7 changes: 4 additions & 3 deletions src/linux_android_with_fallback.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
//! Implementation for Linux / Android with `/dev/urandom` fallback
use crate::{lazy::LazyBool, linux_android, use_file, util_libc::last_os_error, Error};
use crate::{linux_android, use_file, util_libc::last_os_error, Error};
use core::mem::MaybeUninit;
use once_cell::race::OnceBool;

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
// getrandom(2) was introduced in Linux 3.17
static HAS_GETRANDOM: LazyBool = LazyBool::new();
if HAS_GETRANDOM.unsync_init(is_getrandom_available) {
static HAS_GETRANDOM: OnceBool = OnceBool::new();
if HAS_GETRANDOM.get_or_init(is_getrandom_available) {
linux_android::getrandom_inner(dest)
} else {
use_file::getrandom_inner(dest)
Expand Down
7 changes: 4 additions & 3 deletions src/rdrand.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! RDRAND backend for x86(-64) targets
use crate::{lazy::LazyBool, util::slice_as_uninit, Error};
use crate::{util::slice_as_uninit, Error};
use core::mem::{size_of, MaybeUninit};
use once_cell::race::OnceBool;

cfg_if! {
if #[cfg(target_arch = "x86_64")] {
Expand Down Expand Up @@ -94,8 +95,8 @@ fn is_rdrand_good() -> bool {
}

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
static RDRAND_GOOD: LazyBool = LazyBool::new();
if !RDRAND_GOOD.unsync_init(is_rdrand_good) {
static RDRAND_GOOD: OnceBool = OnceBool::new();
if !RDRAND_GOOD.get_or_init(is_rdrand_good) {
return Err(Error::NO_RDRAND);
}
// SAFETY: After this point, we know rdrand is supported.
Expand Down
2 changes: 0 additions & 2 deletions tests/rdrand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
use getrandom::Error;
#[macro_use]
extern crate cfg_if;
#[path = "../src/lazy.rs"]
mod lazy;
#[path = "../src/rdrand.rs"]
mod rdrand;
#[path = "../src/util.rs"]
Expand Down

0 comments on commit 6aaba2f

Please sign in to comment.