bluerhinos / phpMQTT

a simple php class to connect/publish/subscribe to a MQTT broker
Other
773 stars 460 forks source link

Error in writing to socket is not handled. #115

Open jwortelboer opened 4 years ago

jwortelboer commented 4 years ago

When the fwrite on line 435 failes, the 'return false' is unhandled. (no call to _fwrite handles the return value) In my case i had a 'broken pipe' socket error, resulting in an endless loop.

anhlephuoc commented 4 years ago

This happens usually when the Broker drops the connection on purpose and for good reason. Returning a false is not useful because this is called from many different places, and it is very inconvenient to recover from there. Reconnecting(warm) in this function is not practical either because the Broker will only drop the connection again. I suggest that an Exception be thrown here to let the app/developer rectify the problem higher up or abort the program to correct the incompatible parameters requested from the Broker.

jwortelboer commented 4 years ago

I can only spot two places where _fwrite is called. In other places the normal fwrite is called. Not sure why this difference is made here. I agree that some error checking (/throwing) needs to be done. Where the issue is eventually resolved is irrelevant to this issue at this moment in my oppinion. In my case (following the example code) the broken pipe error is detected but ignored and propagates, resulting in an endless loop. It looks like i can reproduce the issue so maybe i can come up with a simple fix.

anhlephuoc commented 4 years ago

The problem actual occurs at fwrite() not just $this->_fwrite(). The false is returned when fwrite() face the condition: invalid stream ID, physical connection error, connection closure by local or remote peer. There are 7 fwrite()s in the code at the moment, and that will grow when(if?) the code is enhanced to support the other packet types. All the fwrite()s need to be protected because we don't know when the errors will come. Without changing any of this code(phpMQTT.php), your client program can redirect the error handler to redirect the warning associated with this error to throw an exception that you can catch in your program. For example add the following code to your client program and put your infinite loop with proc() inside a try {..} catch {..}:

class AppException extends \Exception { public function construct($message, $code = 0, Exception $previous = null) { parent::construct($message, $code, $previous); } } set_error_handler(function ($code, $message, $file, $line) { throw new AppException($message, $code); });

Hao-Wu commented 3 years ago

In my case, I was repeatedly confronted with error message "php fwrite(): send of 8192 bytes failed with errno=11" while the client was trying to write LARGE size file (a json about 200KB) into the tcp socket.

errno 11 indicates the tcp connection which fwrite() being writing is not available. What is interesting is fwrite() will re-try to write to the tcp until the socket is ready (or timeout).

So I guess all this is caused by the Non-blocking mode that set in code line https://github.com/bluerhinos/phpMQTT/blob/7a9d6b26f19e0298e0bef8f873038ab823fb79af/phpMQTT.php#L159

I just modify this line to set the socket stream to be in blocking mode stream_set_blocking($this->socket, true);

With the blocking mode, the fwrite() loops writes to the tcp socket after socket is ready. So errno 11 is solved.