docs: Clarified the point about changing the file descriptor underlyi… #3430
Conversation
Darksonn
added
A-tokio
tokio/src/io/async_fd.rs
Outdated
| /// descriptor results in unspecified behavior in the IO driver, which may | ||
| /// include breaking notifications for other sockets/etc. | ||
| /// descriptor through the underlying [`AsRawFd::as_raw_fd`] results | ||
| /// in unspecified behavior in the IO driver, which may include breaking | ||
| /// notifications for other sockets/etc. That is not to say that simply using | ||
| /// a clone of the inner object would be problematic if that is a supported | ||
| /// thing to do. |
There was a problem hiding this comment.
I don't really like this wording. Some notes:
- The fd is not changed through
as_raw_fd. - I'm not sure we have to expand on the kinds of ways it may break.
- I don't want to make promises around clones of the fd. Those can easily get you in trouble too.
There was a problem hiding this comment.
Isn't the point that as_raw_fs exposes the actual raw file descriptor?
If clones of File might cause UB, then we have a problem (and probably new should be unsafe).
Notwithstanding the huge number of ways in which things can break, there is a definitely a need to provide more guidance around common use cases.
There was a problem hiding this comment.
To clarify the first point, if the raw file fd wasn't exposed, would there be a problem?
There was a problem hiding this comment.
I mean, the AsyncFd access the fd through as_raw_fd, but you don't change it using as_raw_fd. And the kinds of trouble are unspecified behavior, not undefined behavior. My second point was referring to "which may include breaking notifications for other sockets/etc."
There was a problem hiding this comment.
To clarify, the problem happens when as_raw_fd returns different values when called multiple times, but you don't make it return different values through as_raw_fd — it doesn't provide mutable access to the fd.
There was a problem hiding this comment.
It's a statement about the implementer of AsRawFd - if that does something strange, then you have a problem? I'm not sure of the right way of phrasing "done by but not done right here" which was the intention of "through the". How about "Changing the file descriptor through an unexpected implementation of AsRawFd..."?
Perhaps you could clarify the difference between unspecified behaviour and undefined behaviour...?
There was a problem hiding this comment.
One possible way to word it would be:
The inner type must always return the same file descriptor, even if
as_raw_fdis called multiple times. Violating this requirement can cause unspecified behavior.
Unspecified behavior means it does weird stuff. Undefined behavior means unsafe code gone wrong.
There was a problem hiding this comment.
Or maybe reverse the sentence.
The
as_raw_fdmethod on the inner type must always return the same file descriptor when called multiple times.
There was a problem hiding this comment.
I like the last one. I'll do that...
Co-authored-by: Alice Ryhl <alice@ryhl.io>
|
Thank you for the PR! |
…ng an AsyncFd
Motivation
The info on when it's appropriate to have a clone of an underlying
Fileobject when using anAsyncFdwas not clear to me from the current docs.Solution
Adds a clarification point. This may need further clarification, which I'm happy to do.