amphp / byte-stream

A non-blocking stream abstraction for PHP based on Amp.
https://amphp.org/byte-stream
MIT License
363 stars 31 forks source link

Prevent infinite loop on writing to broken pipes #70

Closed dangoodman closed 4 years ago

dangoodman commented 4 years ago

The assertion "Trying to write on a previously fclose()'d resource. Do NOT manually fclose() resources the loop still has a reference to." happens to me even if I have not a single fclose(), stream_XXX() or socket_XXX() call in my code.

My client and server thin wrappers around Server::listen() and connect() are running with Docker containers on Windows. So it might be related to docker networking internals, breaking sockets under some circumstances.

It happens occassionaly. I spent a whole day to find steps to reproduce it with no luck.

Anyway, since the case does really happen sometimes in runtime, I think it would be better to handle it and prevent the infinite loop.

trowski commented 4 years ago

I assume you're using PHP 7.4.0 or 7.4.1? If so, this is a known bug that was fixed in 7.4.2.

dangoodman commented 4 years ago

Right, PHP 7.4.1. Still get the error with PHP 7.4.2 though.

Found a way to reproduce the issue: Test server Test client Example output of test-client.php with PHP 7.1-7.4.2 Example output of test-server.php with PHP 7.4.2 and small chunk size Example output of test-server.php with PHP 7.1-7.3 and small chunk size Example output of test-server.php with PHP 7.1-7.4.2 and regular chunk size

The issue is seemingly related to the read chunk size. When it's set to a small value, e.g. 3 bytes, write to the socket fails.

Also, a similar issue happens to PHP 7.1-7.3. It throws SocketException with message 'Failed to write to stream after multiple attempts; fwrite(): send of XX bytes failed with errno=32 Broken pipe'. Although it seems to be a legit condition, it disappears once we get back to the regular read chunk size, just as with 'Trying to write on a previously fclose()'d resource'.

Not sure what's causing it and whether there is an implied limit on socket read chunk size.

trowski commented 4 years ago

Can you please try with the fix-eagain branch and see if you still get assertion failures on 7.4.2 with small chunk sizes?

dangoodman commented 4 years ago

I tried it with my commit. It doesn't seem to be different to the fix-eagain, if keep PHP 7.4.2 in mind. Am I missing something?

trowski commented 4 years ago

Yes, it was nearly the same as your commit. I pushed another commit to the branch, since it appears EPIPE now returns false in 7.4.2. Not sure if that's related to your issue, so could you now try the fix-eagain branch?

dangoodman commented 4 years ago

With PHP 7.4.2, fix-eagain works now as with PHP 7.1-7.3, i.e. emits SocketException 'fwrite(): send of 23 bytes failed with errno=32 Broken pipe'.

kelunik commented 4 years ago

Closing in favor of #71. Thanks!

dangoodman commented 4 years ago

Thanks.

How about an issue with with PHP 7.1-7.3?

It throws SocketException with message 'Failed to write to stream after multiple attempts; fwrite(): send of XX bytes failed with errno=32 Broken pipe'. Although it seems to be a legit condition, it disappears once we get back to the regular read chunk size, just as with 'Trying to write on a previously fclose()'d resource'.

The issue can be reproduced and fixed in the same way, mentioned above.

Should I post another issue?