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.

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

Module docs for std::os::unix::io inconsistent with I/O Safety discussion in std::io #124384

Closed
jsgf opened this issue Apr 25, 2024 · 2 comments · Fixed by #124412
Closed

Module docs for std::os::unix::io inconsistent with I/O Safety discussion in std::io #124384

jsgf opened this issue Apr 25, 2024 · 2 comments · Fixed by #124412
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@jsgf
Copy link
Contributor

jsgf commented Apr 25, 2024

Location

https://doc.rust-lang.org/stable/std/os/unix/io/index.html

Summary

In std::io, it correctly says:

Note that exclusive ownership of a file descriptor does not imply exclusive ownership of the underlying kernel object that the file descriptor references (also called “file description” on some operating systems). File descriptors basically work like Arc: when you receive an owned file descriptor, you cannot know whether there are any other file descriptors that reference the same kernel object. However, when you create a new kernel object, you know that you are holding the only reference to it. Just be careful not to lend it to anyone, since they can obtain a clone and then you can no longer know what the reference count is! In that sense, OwnedFd is like Arc and BorrowedFd<'a> is like &'a Arc

However the table in std::io::unix::io has the table saying:

BorrowedFd<'a> => &'a _
OwnedFd => Box<_>

This is incorrect and potentially very misleading, because the result of dupping a file descriptor creates a new fd which shares the same underlying file structure in the kernel. In particular, for seekable fds they share the current file offset (ie, used by read write and lseek) and in some cases, file locking state. (In addition to inode state and underlying file/pipe/etc contents.)

The language here should reflect the language in std::io.

@jsgf jsgf added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Apr 25, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 25, 2024
@jsgf
Copy link
Contributor Author

jsgf commented Apr 25, 2024

cc @RalfJung @joshtriplett - I think this is something that #114780 should have addressed.

@RalfJung
Copy link
Member

Yeah the model described in os::unix::io doesn't take into account dup and the indirection layer of "file descriptors" vs "file descriptions". (That was already the case before #114780 though, it just became a lot more apparent with that PR.)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 2, 2024
io safety: update Unix explanation to use `Arc`

Fixes rust-lang#124384

Cc `@jsgf`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 2, 2024
io safety: update Unix explanation to use `Arc`

Fixes rust-lang#124384

Cc ``@jsgf``
fmease added a commit to fmease/rust that referenced this issue May 3, 2024
io safety: update Unix explanation to use `Arc`

Fixes rust-lang#124384

Cc ``@jsgf``
@bors bors closed this as completed in 9ab5cfd May 3, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue May 3, 2024
Rollup merge of rust-lang#124412 - RalfJung:io-safety, r=Amanieu

io safety: update Unix explanation to use `Arc`

Fixes rust-lang#124384

Cc ```@jsgf```
@saethlin saethlin added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants