amphp
Remove requirement to suppress warning
In order to not waste resources on suppressing an E_WARNING and to accommodate debuggers that may ignore the suppression, we can use stream_select to check to see if there is a read available before we attempt to accept the socket.
socket_select ( array &$read , array &$write , array &$except , int $tv_sec [, int $tv_usec = 0 ] ) : int
From the php docs on the &$read parameter:
The sockets listed in the read array will be watched to see if characters become available for reading (more precisely, to see if a read will not block - in particular, a socket resource is also ready on end-of-file, in which case a socket_read() will return a zero length string).
The warning about using === for comparisons is when checking for an error, in this case, we really only want to continue if we have a truthy value. The function returns FALSE for errors OR the (int) number of socket resources contained in the modified arrays.
Tested on php7.4/Linux
Loop::onReadable does basically that, so if the stream isn't readable, the watcher shouldn't be invoked.
Loop::onReadable does basically that, so if the stream isn't readable, the watcher shouldn't be invoked.
I agree with you, but when used inside this while loop, there is a point at which the stream may not be readable. To be honest, I think if it wasn't an issue at some point, I'd imagine the @ suppression would not have been included.
The obvious solution here is to not use a while loop to accept clients but leave the watcher active and rely in the event loop to invoke the watcher again. However, we still may need to retain the error suppression. We use error suppression when accepting a single client in the socket library: https://github.com/amphp/socket/blob/23036caf1a8a236e908c3289222222a35ded53cf/src/Server.php#L88
I assume this is for a reason? Maybe accepting fails occasionally even when the watcher is activated?
I'm sorry I can't add too much more to this conversation, this is a bit beyond the scope of my knowledge.
We still need the suppression, because of multi-process scenarios, where multiple processes listen on the same socket, indeed. Not sure whether we need the while. If we use an if, there shouldn't be warnings by default.
Ah yes, I knew there was a good reason.
I guess our own error handler is the correct fix (and probably should be used everywhere we were forced to use @).
We've switched out uses of @ across all libraries compatible with AMPHP v3 to use a custom error handler to avoid mis-behaving error handlers.
For AMPHP v2, this is far from the only usage of @ and we are not planning on back-porting this "feature." As such, I'd recommend fixing any error handlers to properly check error_reporting().