Use Once instead of lazy_static #790
Conversation
I agree to license my contributions to each file under the terms given at the top of each file I changed.
| use std; | ||
|
|
||
| pub fn fill(dest: &mut [u8]) -> Result<(), error::Unspecified> { | ||
| use lazy_static::lazy_static; | ||
| static OPEN: Once<Result<std::fs::File, std::io::Error>> = Once::new(); |
There was a problem hiding this comment.
Here, wouldn't it make more sense to use std::sync::Once instead of spin::Once? We really don't want threads spinning while we wait for the file open operation to complete (and we can't try to open the file more than once concurrently, because we want to minimize the number of file handles we try to acquire).
| Mechanism::DevURandom => super::urandom::fill(dest), | ||
| static SYSRAND_SUPPORTED: Once<bool> = Once::new(); | ||
| if *SYSRAND_SUPPORTED.call_once(sysrand_supported) { | ||
| super::sysrand::fill(dest) |
There was a problem hiding this comment.
On Linux, at least, getrandom() can block a long time waiting for the kernel entropy pool to be initialized. It seems wrong to have all threads spinning while we wait for the syscall to complete. In theory all these spinning userspace threads could take away CPU time from the kernel and/or other processes that would be useful for initializing the entropy pool and allowing the kernel to return to userspace.
How about, instead, we keep track of whether getrandom() is supported using a tri-state atomic usize:
if 0, call getrandom() to see if it is available, atomically set the flag to 1 or 2 depending on the result, and continue as follows.
If 1, use sysrand::fill() to service the request.
if 2, use urandom::fill() to service the request.
The effect of this would be to push the mutex down to the kernel, out of this userspace process. Presumably the kernel isn't going to use spin locks in such a way that would cause it to start itself of CPU time needed to initialize the entropy pool.
WDYT?
|
Closing this as sticking with |
For RNG detection, we are not really in need of lazy initialization. We're really just trying to cache the result of some function call (in this case
openandsysrand::chunk). This is similar to what we do when detecting CPU features, so using Once inrandas well makes sense.This change lets us eliminate a dependency and allows for consistency with potential future runtime checks in
rand(this would be for platforms like SGX and UEFI that don't havestd).The downside is that it is potentially less efficient on platforms with
std::sync::Once. This shouldn't be an issue in practice, but if it is we can globally switch between the Once implementations based onuse_heap.