MSRV(future): Use std::{sync::OnceLock/io::Read}` to eliminate more unsafe code & once_cell dependency. #485
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
briansmith
commented
Jun 18, 2024
| // https://github.com/rust-lang/rust/issues/126239. | ||
| static FILE: OnceLock<File> = OnceLock::new(); | ||
| let mut file: &File = FILE.get_or_try_init(init)?; | ||
| file.read_buf_exact(BorrowedBuf::from(dest).unfilled()) |
There was a problem hiding this comment.
It seems like we should consider changing the internal getrandom_inner(dest: &mut [MaybeUninit<u8>]) API to take BorrowedCursor/BorrowedBuf instead. util_libc::sys_read_exact could then be changed similarly.
There was a problem hiding this comment.
I think it's a good idea to eventually migrate to BorrowedBuf, but until it's stabilized, I don't think we should change the current approach.
briansmith
changed the title
briansmith
force-pushed
the
b/std-3
branch
2 times, most recently
from
ad02284 to
7b8173e
Compare
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 previous thread. 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 vlaue. 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`.
For now, still use `libc::{poll,read}`. But use `File::open` to open
the files, instead of using `DropGuard`.
While doing this, switch to the `RawFd` type alias from `libc::c_int`.
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.
I tried to use `std::sync::Once` to avoid adding an external dependency
but it doesn't support fallible initialization. `OnceLock` wasn't added
until 1.70, and even then `OnceLock::get_or_try_init` is still
unstable.
Replace some unsafe code with safe code. Eliminate the libc dependency on non-Android/Linux targets that use use_file.
Closed
|
Closing this since it is just a PoC and can't be merged for a long time, if ever. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With these changes, the only
unsafein use_file is Linux/Android's use oflibc::poll.This cannot be merged as-is because it depends on unstable features. This PR is intended to demonstrate potential future directions.