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

AsyncReadExt::read_exact should retry ErrorKind::Interrupted #6162

Open
calmofthestorm opened this issue Nov 20, 2023 · 3 comments
Open

AsyncReadExt::read_exact should retry ErrorKind::Interrupted #6162

calmofthestorm opened this issue Nov 20, 2023 · 3 comments
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-io Module: tokio/io

Comments

@calmofthestorm
Copy link

Version
1.34.0

Platform
Linux alder 6.1.0-9-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.27-1 (2023-05-08) x86_64 GNU/Linux

Description

tokio::io::ReadAsync::read_exact will propagate an std::io::ErrorKind::Interrupted error back to the caller, rather than retrying like its counterpart std::io::Read::read_exact.

I speculate that this is because Interrupted does not commonly come up from non-blocking I/O implementations the way it does for blocking.This is technically working as documented, but I don't think it's working as intended, since implementations of AsyncRead could reasonably assume it is acceptable to return Interrupted if there were a use case for it, and client code is unlikely to handle it since doing so in the std case is unnecessary, and arguably core functionality of read_exact, principle of least surprise, etc.

https://gist.github.com/calmofthestorm/0c95c6f23ed9014238c0d722ee7d97c2 shows how a simple [Async]Read implementation that spuriously returns Interrupted works with std read_exact but not the Tokio equivalent.

@calmofthestorm calmofthestorm added A-tokio Area: The main tokio crate C-bug Category: This is a bug. labels Nov 20, 2023
@Darksonn Darksonn added the M-io Module: tokio/io label Nov 21, 2023
@rcastill
Copy link

I think the same goes for the return type. If it succeeds, it always returns Ok(me.buf.capacity()). In that case, it's better to return io::Result<()> like its counterpart.

@Darksonn
Copy link
Contributor

@rcastill The return type cannot be changed.

@rcastill
Copy link

I realize that. Maybe a comment in the docs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-io Module: tokio/io
Projects
None yet
Development

No branches or pull requests

3 participants