-
-
Notifications
You must be signed in to change notification settings - Fork 14.5k
Implement stdio FD constants #152071
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.
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
Merged
+58
−0
Merged
Implement stdio FD constants #152071
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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 file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| use super::BorrowedFd; | ||
|
|
||
| /// The file descriptor for the standard input stream of the current process. | ||
| /// | ||
| /// See [`io::stdin()`][`crate::io::stdin`] for the higher level handle, which should be preferred | ||
| /// whenever possible. See [`STDERR`] for why the file descriptor might be required and caveats. | ||
| #[unstable(feature = "stdio_fd_consts", issue = "150836")] | ||
| pub const STDIN: BorrowedFd<'static> = unsafe { BorrowedFd::borrow_raw(0) }; | ||
|
|
||
| /// The file descriptor for the standard output stream of the current process. | ||
| /// | ||
| /// See [`io::stdout()`][`crate::io::stdout`] for the higher level handle, which should be preferred | ||
| /// whenever possible. See [`STDERR`] for why the file descriptor might be required and caveats. In | ||
| /// addition to the issues discussed there, note that [`Stdout`][`crate::io::Stdout`] is buffered by | ||
| /// default, and writing to the file descriptor will bypass this buffer. | ||
| #[unstable(feature = "stdio_fd_consts", issue = "150836")] | ||
| pub const STDOUT: BorrowedFd<'static> = unsafe { BorrowedFd::borrow_raw(1) }; | ||
|
|
||
| /// The file descriptor for the standard error stream of the current process. | ||
| /// | ||
| /// See [`io::stderr()`][`crate::io::stderr`] for the higher level handle, which should be preferred | ||
| /// whenever possible. However, there are situations where touching the `std::io` handles (or most | ||
| /// other parts of the standard library) risks deadlocks or other subtle bugs. For example: | ||
| /// | ||
| /// - Global allocators must be careful to [avoid reentrancy][global-alloc-reentrancy], and the | ||
| /// `std::io` handles may allocate memory on (some) accesses. | ||
| /// - Signal handlers must be *async-signal-safe*, which rules out panicking, taking locks (may | ||
| /// deadlock if the signal handler interrupted a thread holding that lock), allocating memory, or | ||
| /// anything else that is not explicitly declared async-signal-safe. | ||
| /// - `CommandExt::pre_exec` callbacks can safely panic (with some limitations), but otherwise must | ||
| /// abide by similar limitations as signal handlers. In particular, at the time these callbacks | ||
| /// run, the stdio file descriptors have already been replaced, but the locks protecting the | ||
| /// `std::io` handles may be permanently locked if another thread held the lock at `fork()` time. | ||
| /// | ||
| /// In these and similar cases, direct access to the file descriptor may be required. However, in | ||
| /// most cases, using the `std::io` handles and accessing the file descriptor via the `AsFd` | ||
| /// implementations is preferable, as it enables cooperation with the standard library's locking and | ||
| /// buffering. | ||
| /// | ||
| /// # I/O safety | ||
| /// | ||
| /// This is a `BorrowedFd<'static>` because the standard input/output/error streams are shared | ||
| /// resources that must remain available for the lifetime of the process. This is only true when | ||
| /// linking `std`, and may not always hold for [code running before `main()`][before-after-main] or | ||
| /// in `no_std` environments. It is [unsound][io-safety] to close these file descriptors. Safe | ||
| /// patterns for changing these file descriptors are available on Unix via the `StdioExt` extension | ||
| /// trait. | ||
| /// | ||
| /// [before-after-main]: ../../../std/index.html#use-before-and-after-main | ||
| /// [io-safety]: ../../../std/io/index.html#io-safety | ||
| /// [global-alloc-reentrancy]: ../../../std/alloc/trait.GlobalAlloc.html#re-entrance | ||
| #[unstable(feature = "stdio_fd_consts", issue = "150836")] | ||
| pub const STDERR: BorrowedFd<'static> = unsafe { BorrowedFd::borrow_raw(2) }; | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be moved to a new section in the
BorrowedFddocs rather than being onSTDERR?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which parts and why? I could see some of it being relevant or at least instructive for general
BorrowedFddocs (e.g., discussion ofBorrowedFd<'static>implications, or thestd::iohandle vs. underlying FD differences, since that also matters for usingAsFdon the handles). But e.g. for the bullet point your comment is attached to, I don't see how it would relate to BorrowedFd docs at all.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
STDOUTandSTDINboth forward readers toSTDERRfor more documentation. This part of the docs could arguably be onSTDOUTorSTDIN, it makes no less or more sense than having it onSTDERR, In cases such as these it does often make sense to have module or type-level.The reason I asked about
BorrowedFdspecifically is that these constants aren't special to std. They can be constructed manually by users, no? So the docs apply equally to those too.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see what you mean now. FWIW, there are some (weak) reasons why I chose
STDERRas the "canonical" place to host the docs: in my experience stderr is the one that's needed most frequently (so documenting it directly saves the most people a click), and both stdin and stdout have unique additional concerns that don't apply to stderr but may be worth discussing (so ifSTDERRreferenced one of the others, some of the referenced docs would not apply to STDERR at all). But if we had a module likestd::os::fd::stdio, I would agree that the docs common to all three constants should live there. That's better than semi-arbitrarily picking one item to host the docs, and the alternatives of duplicating the doc comments or generating them with a macro are really unappealing.However, putting the stdio FD docs on
BorrowedFdfeels weird to me, because I think that the overwhelming majority ofBorrowedFduses don't have anything to do with stdio. Conversely, while you're right that these constants aren't special, I don't think putting these docs (only) onBorrowedFdis likely to reach the users who would benefit from reading them. I think the vast majority of code that ends up handling a stdio fd falls into one of three categories:BorrowedFdfrom the std::io handles (locked or not) viaAsFdimpls. In this case they might check theBorrowedFddocs, but I wouldn't count on it (personally I wouldn't expect to find docs about the stdio case specifically on such a broadly useful type). An explicitly cross-reference in the docs ofimpl AsFd for Std{in,out,err}and/or of the handle types themselves seems useful in any case.rustixornixthat wrap syscalls or libc functions to work with the safe FD types. These libraries either expose and document the stdio FDs themselves (seerustix::stdio), or currently document usage with the std::io handles viaimpl AsFdarguments (e.g., see thenix::unistd::dup2_stdoutexample)BorrowedFdat all, e.g., just useRawFdand the integer constants 0, 1, 2 with libc functions.Many of these uses ought to be able to migrate to the constants added in this PR once they're stable (but not all, e.g. if the locked handles are involved). But IMO the most likely way this will happen is if the constants get cross-referenced by docs or discovered by searching for the terms "stdin" / "stdout" / "stderr". In either case, going to the documentation for the constant and then having to follow a "See this section of
BorrowedFddocs" link for the bulk of the docs feels weird.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've also considered splitting the docs conceptually along the lines of "what to consider when dealing with stdio via FDs" (useful even if you mostly use the handle and sometimes call
as_fd()on it) vs. "why would you need to use the constants" (more esoteric). The former could be useful to document on the std::io handles, but there we face the analogous problem: there's no good place to put docs common to all three handles. The module docs are already very long just surveying the key items and concepts, so adding detailed docs along those lines to the module docs seems like a bad idea.Maybe duplicating docs is the least bad option here. I don't like the risk of docs diverging due to future improvements being applied to one copy and not another, but in
std::io, there's already significant duplication between handle types and the free functions for getting the handles, as well as between different streams (e.g., the Windows portability consideration is repeatedfoursix times betweenstdout(),stderr(),Stdout,Stderr,StdoutLock, andStderrLock).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks. I'm happy to accept this as is for now at least. If someone does have any ideas there's plenty of time for the docs to be adjusted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks. I'm happy to accept this as is for now at least. If someone does have any ideas there's plenty of time for the docs to be adjusted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added as unresolved question to the tracking issue