ghedipunk / PHP-Websockets

A Websockets server written in PHP.
BSD 3-Clause "New" or "Revised" License
913 stars 375 forks source link

socket_write not as robust as originally thought #51

Closed ghedipunk closed 8 years ago

ghedipunk commented 8 years ago

While working on other issues this morning, found that socket_write() is not as robust as I'd originally thought.

function signature:

int socket_write(socket_handle $socket, string $buffer, int $length);

Depending on network conditions and the size of the buffer, it may write less than $length bytes to the socket.

Thus, implementing a _send() and _rawSend() pair of methods: _send() will add the message to a send buffer, and _rawSend() will process the buffer, sending as much of each message as possible. _rawSend() will be implemented in _tick(), as well as called by _send() immediately after putting the message in the buffer.

ghedipunk commented 8 years ago

Also replacing each instance of socket_write() that already exists with _send(), in case it's not already painfully obvious that I'm doing so. ;-)

ghedipunk commented 8 years ago

Passes a smoke test, but could use some more robust testing before closing. I'm moving on to #46 right now, followed by #45, then #47 before coming back to thoroughly test this.

Xaraknid commented 8 years ago

It's all depend if you are in BLOCKING or NON-BLOCKING mode.

By default, TCP sockets are in "blocking" mode. For example, when you call recv() to read from a stream, control isn't returned to your program until at least one byte of data is read from the remote site. This process of waiting for data to appear is referred to as "blocking". The same is true for the write() API, the connect() API, etc. When you run them, the connection "blocks" until the operation is complete.

Of course for having a responsive server we'll need to make sure every socket connection are in NON-BLOCKING mode. Because of that we'll need buffering routine to make sure everything is send/receive as you discover.

I highly recommend that you use write section of socket_select(read,write,execpt). Because server will only send data when the socket connection is ready to receive it. It'll be pointless to call socket_write if 0 bytes can be sended.

As the read flags only appear when you have data on the line, write flags you MUST only watch it when you required to send data unless you want to see 100% CPU, because write flags allways appear to tell you 'hey i'm ready to receive something' for every loop of select unless they are not ready to receive anything.

For example check the development branch core/eventloop/socket.php -For how I set the write flags and core/server/server.php -ws_write method for how I open and close the write flags and adjust data buffer accordingly.

Xaraknid commented 8 years ago

I see a potential bug that will occur if those conditions are met. 1- multiple messages to be sent to one connection require more than 1 packet (socket_write). 2- Enough time between 2 messages to the same connection in the list.

That will corrupt the frame you try to send and will result in client closing the connection. That's because you use parallel technique.

Consider for the same connection you'll have those frame requiring more than one packet pending in your list look like this :

frame #1 (packet A) ... frame #2 ( packet B, C ) ... frame #3 (packet D,E) ... frame #4(packet F,G)

First loop of the list.
Frame #1 write #1-A : send perfectly remove this frame from the list.
Frame #2 not writed because not ready
Frame #3 enough time elapse between connection ready write #3-D 
Frame #4 again enough time elapse connection ready write #4-F  corrupt websocket connection

To prevent that you need to linear them by concatening all frame in one buffer.

Another issue i see is you didn't do any cleanup after a connection close or check for error. That will lead in memory exhaustion and useless use of cpu to process those data that can't be send anymore.

Because socket_write can send 0 for two reasons, nothing was send or an error occur. Everytime you try to send to a close connection you will receive 0 as false so the messages will allways stay in the list.