use_file: Use std::fs::File instead of libc and once_cell::sync::OnceCell instead of pthreads mutex
#481
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
This was referenced Jun 18, 2024
Closed
newpavlov
reviewed
Jun 18, 2024
| } else { | ||
| get_fd_locked() | ||
| } | ||
| File::open(FILE_PATH).map_err(map_io_error) |
There was a problem hiding this comment.
I think we should keep the libc-based open_readonly function. This code is less transparent and IIUC involves adding null termination to the file path. We have to use libc for reading, so using it for opening should be fine as well.
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.
|
Closing it in favor of #490, but we may reconsider it before v0.3 release. |
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.
See the individual commit messages for details.