Skip to content

Fix GH-12104 attempt #14311

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

Closed
wants to merge 1 commit into from
Closed

Conversation

devnexen
Copy link
Member

adding a bit of latency to give time to receive the data.

Copy link
Member

@arnaud-lb arnaud-lb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you consider using select() to wait for data ?

Copy link
Member

@bukka bukka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1s seems too much to me. Can you try something between 50ms and 200ms which should be enough I would think.

@bukka
Copy link
Member

bukka commented May 24, 2024

Ideally it would be better to rewrite with select but guess it might require more time so not asking for that.

@arnaud-lb
Copy link
Member

It may be worth it to try select() if it's a two lines change, as it would make the test faster than with sleep(), and more reliable.

@arnaud-lb
Copy link
Member

Thank you @devnexen! @bukka is it ok for you?

stream_socket_recvfrom($server, 1, 0, $peer);
stream_socket_sendto($server, $data, 0, $peer);
if (stream_select($read, $write, $exc, 0, 250000) === false) die ("stream_select timeout");
Copy link
Member

@bukka bukka May 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't $read be set to [$fd]?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

using stream_select to gives the chance to process the data.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants