Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Structures for lock-free initialization #51

Merged
merged 4 commits into from
Jul 2, 2019
Merged

Add Structures for lock-free initialization #51

merged 4 commits into from
Jul 2, 2019

Conversation

josephlr
Copy link
Member

@josephlr josephlr commented Jul 1, 2019 •
edited
Loading

Fixes #50

This is the first step to improving the synchronization concerns in getrandom. Currently we need synchronization to:

  1. Hold a global file descriptor for /dev/urandom or /dev/random
  2. "Weak linkage" of a libc symbol (getrandom on solaris or getentropy on macos).
  3. Detecting RDRAND
  4. Detecting getrandom(2) on Linux.
  5. Determining the environment on wasm32-unknown-unknown.

Right now we use lazy_static for all of these, but that isn't ideal if we want getrandom to be used to implement libstd. To address some of these concerns, we add a LazyUsize type that just wraps an AtomicUsize, and provides methods for lazy initialization, where it is OK for the initialization to be run multiple times. We then build two structs on top of LazyUsize:

  1. LazyBool - This is just like LazyUsize except with bool types in the function signatures. It is used to replace lazy_static for (3) and (4).
  2. Weak - Based on the libstd struct of the same name, this allows us to "link" to system libc functions that may or may not be present at runtime. This replaces lazy_static for (2). It also simplifies some duplicated logic, avoids locking, and makes implementing Use getrandom on FreeBSD #35 easier.

The remaining uses of lazy_static can be eliminated by just using a combination of std::sync::Once and static mut in a follow up PR (see #52).

@josephlr
Copy link
Member Author

josephlr commented Jul 1, 2019

This is based on @newpavlov's idea here: #50 (comment), it just uses AtomicBool instead of AtomicUsize, and encapsulates the complexity in a struct in a separate file.

dhardy
dhardy reviewed Jul 1, 2019
View reviewed changes
src/util.rs Outdated Show resolved Hide resolved
@newpavlov
Copy link
Member

newpavlov commented Jul 1, 2019 •
edited
Loading

Why do we need to bother with the LazyBool abstraction? I think it will be fine to simply use code in the issue, it's more efficient and not that complex in comparison. (ideally it would use AtomicU8, but it will require bumping MSRV for UEFI and SGX targets)

UPD: Ok, sorry I haven't read the description, only briefly looked at the code.

@josephlr
Copy link
Member Author

josephlr commented Jul 1, 2019

Why do we need to bother with the LazyBool abstraction?

Having the abstraction is crucial if we want to use this logic in multiple places (like this PR does).

newpavlov
newpavlov reviewed Jul 1, 2019
View reviewed changes
src/util.rs Outdated Show resolved Hide resolved
@josephlr josephlr changed the title Add LazyBool for lock-free initialization Add Structures for lock-free initialization Jul 1, 2019
@josephlr
Copy link
Member Author

josephlr commented Jul 1, 2019

I've update the description for this. Now there are two lock-free helpers LazyBool and Weak, both using an AtomicUsize to implement the lazy initialization.

@josephlr josephlr mentioned this pull request Jul 1, 2019
Merged
dhardy
dhardy approved these changes Jul 2, 2019
View reviewed changes
src/util.rs Outdated
// will always be retried on a future call to unsync_init(). This makes it
// ideal for representing failure.
pub fn unsync_init(&self, init: impl FnOnce() -> usize) -> usize {
// Relaxed ordering is fine, as init() is allowed to run mulitple times.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong explanation. Probably you know, but the reason is that we only have a single shared variable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://llvm.org/docs/Atomics.html#atomic-orderings

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh duh, I was overthinking my explanation. Fixed.

I also renamed the lazy module to be util_libc as I'm anticipating we will need more libc based utility functions.

newpavlov
newpavlov reviewed Jul 2, 2019
View reviewed changes
src/util.rs Outdated Show resolved Hide resolved
src/util_libc.rs Outdated Show resolved Hide resolved
@newpavlov newpavlov merged commit b1e8154 into rust-random:master Jul 2, 2019
@josephlr josephlr deleted the lazy branch July 3, 2019 08:51
@dhardy dhardy mentioned this pull request Apr 11, 2021
Open
josephlr added a commit that referenced this pull request Oct 23, 2022
@josephlr
Solaris: consistantly use /dev/random source
b1ec46a 
On Solaris, we opt to use /dev/random source instead of /dev/urandom due
to reasons explained in the comments and
[in this Solaris blog post](https://blogs.oracle.com/solaris/post/solaris-new-system-calls-getentropy2-and-getrandom2).

However, we haven't been making the same choice when getting randomness
via the `getrandom(2)` function, as we just pass `0` for the flags. We
[used to](https://github.com/rust-random/rand/pull/730/files#diff-694d4302a3ff2a976f2fbd34bc05ada22ee61a4e21d2d985beab27f7a809268fR151)
always set `GRND_RANDOM`, but that was removed in the move from `OsRng`
to this crate.

For context, rust-random/rand#730,
#9, and
#51 are the major changes
to the Solaris/Illumos implementation over the years.

See the solaris documentation for:
- [`getrandom(2)`](https://docs.oracle.com/cd/E88353_01/html/E37841/getrandom-2.html)
- [`urandom(4)`](https://docs.oracle.com/cd/E88353_01/html/E37851/urandom-4d.html)

I also updated the doucmentation to better reflect when
[Illumos added the `getrandom(2)` function](https://www.illumos.org/issues/9971#change-23483).

Signed-off-by: Joe Richey <[email protected]>
@josephlr josephlr mentioned this pull request Oct 23, 2022
Merged
josephlr added a commit that referenced this pull request Oct 23, 2022
@josephlr
Solaris: consistantly use /dev/random source
d1f3361 
On Solaris, we opt to use /dev/random source instead of /dev/urandom due
to reasons explained in the comments and
[in this Solaris blog post](https://blogs.oracle.com/solaris/post/solaris-new-system-calls-getentropy2-and-getrandom2).

However, we haven't been making the same choice when getting randomness
via the `getrandom(2)` function, as we just pass `0` for the flags. We
[used to](https://github.com/rust-random/rand/pull/730/files#diff-694d4302a3ff2a976f2fbd34bc05ada22ee61a4e21d2d985beab27f7a809268fR151)
always set `GRND_RANDOM`, but that was removed in the move from `OsRng`
to this crate.

For context, rust-random/rand#730,
#9, and
#51 are the major changes
to the Solaris/Illumos implementation over the years.

See the solaris documentation for:
- [`getrandom(2)`](https://docs.oracle.com/cd/E88353_01/html/E37841/getrandom-2.html)
- [`urandom(4)`](https://docs.oracle.com/cd/E88353_01/html/E37851/urandom-4d.html)

I also updated the doucmentation to better reflect when
[Illumos added the `getrandom(2)` function](https://www.illumos.org/issues/9971#change-23483).

Signed-off-by: Joe Richey <[email protected]>
josephlr added a commit that referenced this pull request Oct 23, 2022
@josephlr
Solaris: consistantly use /dev/random source
21fa45e 
On Solaris, we opt to use /dev/random source instead of /dev/urandom due
to reasons explained in the comments and
[in this Solaris blog post](https://blogs.oracle.com/solaris/post/solaris-new-system-calls-getentropy2-and-getrandom2).

However, we haven't been making the same choice when getting randomness
via the `getrandom(2)` function, as we just pass `0` for the flags. We
[used to](https://github.com/rust-random/rand/pull/730/files#diff-694d4302a3ff2a976f2fbd34bc05ada22ee61a4e21d2d985beab27f7a809268fR151)
always set `GRND_RANDOM`, but that was removed in the move from `OsRng`
to this crate.

For context, rust-random/rand#730,
#9, and
#51 are the major changes
to the Solaris/Illumos implementation over the years.

See the solaris documentation for:
- [`getrandom(2)`](https://docs.oracle.com/cd/E88353_01/html/E37841/getrandom-2.html)
- [`urandom(4)`](https://docs.oracle.com/cd/E88353_01/html/E37851/urandom-4d.html)

I also updated the doucmentation to better reflect when
[Illumos added the `getrandom(2)` function](https://www.illumos.org/issues/9971#change-23483).

Signed-off-by: Joe Richey <[email protected]>
josephlr added a commit that referenced this pull request Oct 23, 2022
@josephlr
Solaris: consistantly use /dev/random source
a524072 
On Solaris, we opt to use /dev/random source instead of /dev/urandom due
to reasons explained in the comments and
[in this Solaris blog post](https://blogs.oracle.com/solaris/post/solaris-new-system-calls-getentropy2-and-getrandom2).

However, we haven't been making the same choice when getting randomness
via the `getrandom(2)` function, as we just pass `0` for the flags. We
[used to](https://github.com/rust-random/rand/pull/730/files#diff-694d4302a3ff2a976f2fbd34bc05ada22ee61a4e21d2d985beab27f7a809268fR151)
always set `GRND_RANDOM`, but that was removed in the move from `OsRng`
to this crate.

For context, rust-random/rand#730,
#9, and
#51 are the major changes
to the Solaris/Illumos implementation over the years.

See the solaris documentation for:
- [`getrandom(2)`](https://docs.oracle.com/cd/E88353_01/html/E37841/getrandom-2.html)
- [`urandom(4)`](https://docs.oracle.com/cd/E88353_01/html/E37851/urandom-4d.html)

I also updated the doucmentation to better reflect when
[Illumos added the `getrandom(2)` function](https://www.illumos.org/issues/9971#change-23483).

Signed-off-by: Joe Richey <[email protected]>
josephlr added a commit that referenced this pull request Oct 23, 2022
@josephlr
Solaris: consistantly use /dev/random source (#310)
f2d7662 
On Solaris, we opt to use /dev/random source instead of /dev/urandom due
to reasons explained in the comments and
[in this Solaris blog post](https://blogs.oracle.com/solaris/post/solaris-new-system-calls-getentropy2-and-getrandom2).

However, we haven't been making the same choice when getting randomness
via the `getrandom(2)` function, as we just pass `0` for the flags. We
[used to](https://github.com/rust-random/rand/pull/730/files#diff-694d4302a3ff2a976f2fbd34bc05ada22ee61a4e21d2d985beab27f7a809268fR151)
always set `GRND_RANDOM`, but that was removed in the move from `OsRng`
to this crate.

For context, rust-random/rand#730,
#9, and
#51 are the major changes
to the Solaris/Illumos implementation over the years.

See the solaris documentation for:
- [`getrandom(2)`](https://docs.oracle.com/cd/E88353_01/html/E37841/getrandom-2.html)
- [`urandom(4)`](https://docs.oracle.com/cd/E88353_01/html/E37851/urandom-4d.html)

I also updated the doucmentation to better reflect when
[Illumos added the `getrandom(2)` function](https://www.illumos.org/issues/9971#change-23483).

Signed-off-by: Joe Richey <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@newpavlov newpavlov newpavlov left review comments

@dhardy dhardy dhardy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Don't use spin_no_std feature
3 participants
@josephlr @newpavlov @dhardy