Description
I propose that on all targets where we know libstd is available, we use libstd instead of pthreads and libc for the use_file
implementation, effectively reverting to the original implementation.
The current proposal for x86_64-unknown-linux-none in #424 is to limit support for that target to Linux kernel versions that have the getrandom
syscall, so it won't use use_file
.
Historically, we've tried to make it so getrandom
avoids using libstd in part so that libstd itself could use getrandom
as a dependency. However, I think it is clear that libstd is not going to use this crate as its needs are almost completely different; indeed its code has diverged considerably.
Another reason we've avoided libstd is to ensure that we are #[no_std]
compatible. However, once we add x86_64-unknown-linux-none support, we'll have an actual no-std target that we can build (at least) for to ensure the core parts of this crate remain no_std
compatible.
use_file
is a significant chunk of the unsafe code, with the most problematic dependencies (libpthreads). It duplicates functionality that's in libstd, and the functionality it duplicates is exactly the kind of functionality (threading primitives) that we should avoid duplicating, as we're not getting any benefit from the QA and improvements in libstd.
Of all the targets that use the use_file
implementation, only one doesn't have libstd support: 32-bit x86 QNX Neutrino. I believe @samkearney is working on it, based on their comment in rust-lang/rust#109173 (comment).
Activity
samkearney commentedon Jun 4, 2024
Thanks for the mention @briansmith, just FYI, I have abandoned the work to bring std support to x86 QNX neutrino 7.0. QNX dropped support for 32-bit architectures in 7.1 (released 2020) and the cost-benefit for adding support to a 7-year-old release doesn't make sense anymore.
That being said, I also don't care if no_std x86 QNX 7.0 breaks, and I would wager pretty strongly that virtually nobody else is using it either (it is much more likely that they are using the more recent QNX 7.1 or the in-progress support for QNX 8).
[-]Just use `libstd` on targets that we know have libstd, instead of pthreads + libc.[/-][+]use_file: Just use `libstd` on targets that we know have libstd, instead of pthreads + libc.[/+]briansmith commentedon Jun 4, 2024
I think we should switch 32-bit x86 QNX to be a target for which we support custom implementations, and drop the built-in implementation for it. That way we don't have to carry around the current implementation forever just for it. Anybody who needs the current implementation could adopt the current
use_file
code in their custom implementation.samkearney commentedon Jun 4, 2024
Works for me. 👍
josephlr commentedon Jun 4, 2024
My long-term goal would still be to have Rust's standard library use
getrandom
, but that requires #365. Upstream seemed fairly open to it last time we tried, and it would be nice to have this crate be officially part of the Rust project. However, even if this crate were used by libstd:poll
ing of/dev/random
wouldn't be part of thatMutex
andFile
abstractions of libstd and not roll our own.So I think this issue is still worth considering, even if we do try to get this into libstd one day.
Do you know if there would be a way to
poll
the/dev/random
file descriptor usingstd
, or would we need to still uselibc
for that?briansmith commentedon Jun 4, 2024
I agree with your "even if this crate were used by libstd...".
I think we'd still need to use
AsFd::as_fd
(1.63+) oras_raw_fd()
to get the file descriptor and poll it with libc. But, only on Linux/Android.use_file: `std::sync::Mutex`, dropping all libpthread use.
use_file: `std::sync::Mutex`, dropping all libpthread use.
std::sync::Mutex
, dropping all libpthread use. #457use_file: `std::sync::Mutex`, dropping all libpthread use.
use_file: `std::sync::Mutex`, dropping all libpthread use.
use_file: `std::sync::Mutex`, dropping all libpthread use.
use_file: `std::sync::Mutex`, dropping all libpthread use.
use_file: `std::sync::Mutex`, dropping all libpthread use.
use_file: `std::sync::Mutex`, dropping all libpthread use.
35 remaining items