Repository navigation
Do not report ECANCELED under a pending exception as a stream failure - #3
Merged
Merged
Conversation
A plain-wrapper read or write, or a socket read, returns ECANCELED when its op was abandoned for an exception: the provider's run() failed, or the op never started because the exception was already pending. The wrappers took that for a failure: a notice on top of the exception, and eof set on a stream that is not at its end. The socket write path and the TLS wrapper already stay quiet in that case. ECANCELED without a pending exception, a provider completing the op as Cancelled, stays an error.
Once a parked accept armed the multishot accept of a listener, the ring moved new connections from the kernel into the record's buffer, and stream_select() and socket_select() polled the kernel only: a select and a non-blocking accept loop waited its whole timeout with connections pending. A Poll for reading on such a listener is now answered from the buffer, or parks on the record until the multishot buffers a connection or ends. A non-blocking accept with an empty buffer no longer arms the multishot: the direct one completes Unsupported so the core calls accept(), which also makes it work on the threads backend, where a chain whose deadline is due never starts.
Owner
|
Thanks this is now integrated with the more generic fix for the select / multishot bit in 67af13e |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Do not report ECANCELED under a pending exception as a stream failure
When an op is abandoned because of an exception (the provider's
run()fails, orphp_io_run_ex()does not start it because an exception is already pending), the wrappers get -1 witherrno == ECANCELED.plain_wrapper.c(read and write) andxp_socket.c(read) treat that as a real failure:Read of 8192 bytes failed with errno=125 Operation canceled(orWrite of …) on top of the exception;eofset on a stream that is not at its end, so after catching the exceptionfeof()is true and the nextfgets()on the pipe returns false.The socket write path already skips its warning under
EG(exception), and xp_ssl treats ECANCELED as a retry, so the plain and socket read paths were the odd ones out.Fix: skip the notice and
eofonly for ECANCELED withEG(exception)set. ECANCELED without an exception (a provider completing the op as Cancelled, a kernel -ECANCELED from the ring) stays an error.php_io_hooks.hnow documents both meanings.Tests:
ext/standard/tests/streams/hooks/data-ops-cancel-socket-not-eof.phptanddata-ops-cancel-pipe-not-eof.phpt. Both fail onio_hooks_poc(608927e) and pass with the fix;ext/standard/tests/streams,fileandext/socketspass.