Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error in writing to socket is not handled. #115

Open
jwortelboer opened this issue Nov 5, 2020 · 4 comments
Open

Error in writing to socket is not handled. #115

jwortelboer opened this issue Nov 5, 2020 · 4 comments

Comments

@jwortelboer
Copy link

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
Copy link

anhlephuoc commented Nov 6, 2020

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
Copy link
Author

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
Copy link

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
Copy link

Hao-Wu commented Apr 28, 2021

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

stream_set_blocking($this->socket, 0);

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants