Conversation
bukka
left a comment
There was a problem hiding this comment.
As I just noted in #23879, this is not a bug. So it should target master.
I'm also not sure why it does it only for non-blocking. It shouldn't really matter that much. Also there is a bit semantic change in php_stream_xport_accept() return value so that should probably be noted in UPGRADING.INTERNALS (so it couldn't probably target 8.6 in any case).
|
I rebased onto master and dropped the non-blocking check in 5178927, so a timeout with no pending connection is now silent on blocking listeners too. The UPGRADING.INTERNALS note for |
|
Hmm I think it should still produce error if it's not a 0 timeout. It might make sense to change that error to TimeOut . Only the 0 timeout would be ignored then. |
|
I changed it in 1cd4470. A 0 timeout with no pending connection is now silent. When a non-zero timeout expires, |
As I replied there, I disagree on that and believe it makes a new feature pretty much unusable in practice. It looks like this PR can resolve the issue without introducing any API / ABI change, making it theoretically applicable to PHP 8.6. I've requested PHP 8.6 RM review for them to decide. |
|
I think we need to first agree whether it is a bug or feature. RM cannot decide this and we require full agreement on such topic. Currently we don't have such agreement - let's keep that discussion in the actual issue so we don't duplicate it here. But even if we agreed that this this is a bug, there is, however, still semantic internal API change that this would introduce. It changes the php_stream_xport_accept() contract to return 0 with a NULL client and it currently changes the reported code for non zero timeouts |
When
stream_socket_accept()timed out with no client waiting, it went down the same path as a real failure and raisedAcceptFailed("Connection timed out"). That makes the usual "accept until empty" drain loop with a 0 timeout throw inStreamErrorMode::Exception, and the only way to tell a timeout apart from something like fd exhaustion was the message text.The TCP and TLS transports now treat a poll timeout or
EAGAINas "nothing to accept" and return success without a client.stream_socket_accept()then returnsfalsewith no error for a 0 timeout, and reportsTimeOutinstead ofAcceptFailedwhen a non-zero timeout expires. Real accept failures still reportAcceptFailed.PHP_TIMEOUT_ERROR_VALUEmoved fromnetwork.ctophp_network.hso the transports can check for it.Closes #23879