EINTR handling

[sunshowers]

I wanted to bring EINTR (ErrorKind::Interrupted) handling up as a general topic.

Traditionally, EINTR on Unix systems has meant that a signal has interrupted this syscall. Usually, the operation can be immediately retried (unlike EAGAIN/EWOULDBLOCK/WouldBlock which would have to be retried later).

How should tokio-io handle EINTR, if at all? For example, this call in FramedWrite::poll_complete can fail with ErrorKind::Interrupted -- this means that a stream forwarding to a FramedWrite can fail with EINTR at this point. This means that a Forward future when run with Core::run will fail and consume the stream and sink, even though it can just be immediately retried.

• Should FramedWrite retry on ErrorKind::Interrupted?
• Should try_nb! retry on ErrorKind::Interrupted?

More general questions (perhaps more appropriate for other crates):

• Should Forward futures have some way to return the stream and sink on error as well?
• How should this general pattern (Core::run gets passed a future which errored out, but the future is still valid and poll can still be called on it) be handled?

[carllerche]

I would think that EINTR should probably be swallowed in the concrete implementations like TcpStream I would think? Then no special handling would be needed in tokio-io. I'm interested in the thoughts of others re this topic

…

On Sat, Apr 8, 2017 at 8:06 PM Siddharth Agarwal ***@***.***> wrote: I wanted to bring EINTR (ErrorKind::Interrupted) handling up as a general topic. Traditionally, EINTR on Unix systems has meant that a signal has interrupted this syscall. Usually, the operation can be immediately retried (unlike EAGAIN/EWOULDBLOCK/WouldBlock which would have to be retried later). How should tokio-io handle EINTR, if at all? For example, this call <https://github.com/tokio-rs/tokio-io/blob/d48c506a3edd13bc5b0bfa5e49bb53021d37c07c/src/framed_write.rs#L201> in FramedWrite::poll_complete can fail with ErrorKind::Interrupted -- this means that a stream forwarding to a FramedWrite can fail with EINTR at this point <http://alexcrichton.com/futures-rs/src/futures/stream/forward.rs.html#80>. This means that a Forward future when run with Core::run will fail and consume the stream and sink, even though it can just be immediately retried. - Should FramedWrite retry on ErrorKind::Interrupted? - Should try_nb! and try_ready! retry on ErrorKind::Interrupted? More general questions (perhaps more appropriate for other crates): - Should Forward futures have some way to return the stream and sink on error as well? - How should this general pattern (Core::run gets passed a future which errored out, but the future is still valid and poll can still be called on it) be handled? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#37>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAYJMu9_A8AFiokd75XCu6qCzK-vqCWks5ruEtIgaJpZM4M35zQ> .

[sunshowers]

(Whoops didn't mean to close this)

That would require every AsyncRead and AsyncWrite to do its own EINTR handling, which means like much of the standard library like File would have to be wrapped.

[alexcrichton]

Yeah the intention is definitely that no one worries about EINTR handling in general. The standard library chose to expose EINTR from read and write functions but not higher level conveniences like read_to_end and such.

I think it's reasonable to follow a similar pattern in tokio where all combinators and such handle Interrupted, but the raw functions don't handle it (as they're just proxying to libstd)

[carllerche]

Do we even want to have EINTR in read / write. I see how this makes sense for std which is providing an API to use without a runtime but with tokio you are running in context of an executor and you probably don't want the thread to hit interrupts in the first place.

I was thinking of swallowing EINTR in Read / write impls on TcpStream.

[sunshowers]

OK, so it turns out that tokio-io must handle EINTR. Here's why:

1. The documentation for Write::write says that

A call to write represents at most one attempt to write to any wrapped object.

2. This means that if one has a buffered writer and the buffer is full, the only valid thing to do on write is to write out the internal buffer, then return an error which tells the caller to retry.

3. The only error universally recognized as "retry immediately" is ErrorKind::Interrupted.

[sunshowers]

That's what zstd-rs does here. (flate2-rs and bzip2-rs are actually in violation of the contract for Write::write -- I'll file issues for both of them shortly.)

[sunshowers]

I've filed alexcrichton/bzip2-rs#18 and alexcrichton/flate2-rs#93.

[alexcrichton]

Thanks for filing issues @sid0!

Note that the comment in the docs there was largely targeted at atomicity. Basically if you successfully write any bytes then that must be reported back. I think it's fine, semantically, for an auto-retry to happen on EINTR.

@carllerche you bring up a good point, tokio is less libstd-esque. I'd be fine auto-retrying on EINTR on TcpStream/TcpListener/UdpSocket.

[sunshowers]

Basically if you successfully write any bytes then that must be reported back. I think it's fine, semantically, for an auto-retry to happen on EINTR.

So this wasn't clear to me from the docs at all. If that is truly the intent then the docs should probably be updated to reflect that.

I've opened up a topic on the internals board about that: https://internals.rust-lang.org/t/std-io-write-a-call-to-write-represents-at-most-one-attempt-to-write-to-any-wrapped-object/5068

[carllerche]

What is the conclusion of this issue? Are there any action items?

[sunshowers]

Yeah -- it should be documented that AsyncWrite instances should never propagate ErrorKind::Interrupted errors and should instead retry.

[carllerche]

Would you be able to write up a PR with the doc changes and maybe check if tokio-core handles EINTR?

[sunshowers]

Would you be able to write up a PR with the doc changes and maybe check if tokio-core handles EINTR?

Probably not any time soon, I'm afraid.

[carllerche]

The tokio-io crate has been moved into the tokio git repo.

I created a new issue over there. See link above.