Only close stream when the we reached end of the stream #139
Conversation
|
Please note that this PR doesn't contain actual code change at the moment. I'll create tests first and commit them before committing the fix to show tests work appropriately. |
WyriHaximus
changed the title
tests/ReadableResourceStreamTest.php
Outdated
| { | ||
| $loop = $this->createLoopMock(); | ||
|
|
||
| $conn = new ReadableResourceStream(STDIN, $loop); |
There was a problem hiding this comment.
The test for DuplexResourceStream LGTM, can you change this one to also avoid using any external resources?
STDIN is stateful and this will change it to non-blocking mode which will persist even after the test suite has terminated.
There was a problem hiding this comment.
I'll look into that 👍
There was a problem hiding this comment.
And I recall again why I used STDIN, it was the quickest working stream available xD
| { | ||
| list($stream, $_) = stream_socket_pair(STREAM_PF_UNIX, STREAM_SOCK_STREAM, 0); |
There was a problem hiding this comment.
This is used throughout the rest suite. If this breaks on windows a follow up PR for all of them seems more appropriate to be honest
WyriHaximus
force-pushed
the
ext-uv-feof
branch
from
6eb23e0 to
5b9d87d
Compare
|
Squashed everything into one commit, for those interested the original history can be found at: https://github.com/WyriHaximus-secret-labs/stream/tree/ext-uf-feof-history |
Construct underlying stream to always consume complete receive buffer. This avoids stale data in TLS buffers and also works around possible buffering issues in legacy PHP versions. The buffer size is limited due to TCP/IP buffers anyway, so this should not affect usage otherwise. This builds on top of reactphp/stream#139 to work around a bug in PHP where reading from a TLS 1.3 stream resource would hang with 100% CPU usage due to the changed TLS 1.3 handshake.
While working on reactphp/event-loop#112 I ran into an issue with suddenly closed streams. Traced the issue down to
''coming from streams when using theext-uvloop. Adding this check before closing the stream resolved the issue forext-uv.Todo:
Relates to reactphp/event-loop#112