Non-blocking socket reading

[Seriyyy95]

I am trying to fix the bug in chrome-php/chrome with Dom class, described in this issue. I need ability to read events from the socket without blocking execution. But I not sure should I refactor existing code or add new method.

I prepared required changes here and here

[stale]

This issue has been automatically marked as stale because there has been no recent activity. It will be closed after 30 days if no further activity occurs. Thank you for your contributions.

[enricodias]

I think that a non-blocking read is a nice feature. fgetc isn't even needed, a normal freadwould do. Duplicating code isn't the best way to implement this, the read and receive methods are almost identical. The code in the old method isn't that good either and could be simplified.

Listening for the documentUpdated in the chrome package is a different feature specific to that package. Note that if happens to read an incomplete message, the last event in that message will be lost. This is not a concern for wrench itself, though.

[Seriyyy95]

I think that a non-blocking read is a nice feature. fgetc isn't even needed, a normal freadwould do. Duplicating code isn't the best way to implement this, the read and receive methods are almost identical. The code in the old method isn't that good either and could be simplified.

Listening for the documentUpdated in the chrome package is a different feature specific to that package. Note that if happens to read an incomplete message, the last event in that message will be lost. This is not a concern for wrench itself, though.

Thank you for your reply. I can refactor the receive method. In which case it can fetch an incomplete message? When length is smaller than the message size?

The method can't always be non-blocking because it should wait for the first message from WebSocket on handshake in Wrench\Client::243. Also, some tests expect blocking behavior. So, I added the $blocking flag with the default value true. When I need non-blocking, I will call the method $blocking = false

Additionally, the current implementation waits for a 5 sec timeout for the first message. I can optimize it and return the message as soon as possible so the app will start much faster.

[enricodias]

Thank you for your reply. I can refactor the receive method. In which case it can fetch an incomplete message? When length is smaller than the message size?

Exactly.

The method can't always be non-blocking

We don't need to change the original method behavior, we just need to make them share the parts of the code that are identical in both. Just o prevent code duplication since blocking vs non blocking will have just a few different lines.

Additionally, the current implementation waits for a 5 sec timeout for the first message. I can optimize it and return the message as soon as possible so the app will start much faster.

In which line did you see this?

[Seriyyy95]

In which line did you see this?

Well, I can't reproduce it now with the original method. Maybe I saw this in one of my implementations with getc. The max handshake length is set to 1500 bytes, but the actual handshake size is near 138 bytes, and the socket has 5 sec timeout. I thought that when the fread() function reads handshake, it waits for timeout because the length is much smaller than the limit. However, fread returns the handshake response immediately when it becomes available. So there are no issues.

I will write a few tests for the original method to have more info about its behavior and then try refactoring it.

[stale]

This issue has been automatically marked as stale because there has been no recent activity. It will be closed after 30 days if no further activity occurs. Thank you for your contributions.

[divinity76]

In the (not yet released) 1.7 branch, \Wrench\Client has a new public function waitForData(float $maxSeconds): ?bool method, you can do waitForData(0) to check if data is available before reading (0 is a special-case for "don't wait for data, just check if there is data"), that should allow you to effectively read non-blockingly 👍 see #17 for details

[Seriyyy95]

Great! That may work. I busy these days at work, try to look at this on weekends.

[Seriyyy95]

In the (not yet released) 1.7 branch, \Wrench\Client has a new public function waitForData(float $maxSeconds): ?bool method, you can do waitForData(0) to check if data is available before reading (0 is a special-case for "don't wait for data, just check if there is data"), that should allow you to effectively read non-blockingly 👍 see #17 for details

Thanks for adding waitForData(). It completely solves this issue. I fixed that bug with node id locally and am ready to send a pull request for chrome-php/crhome.