Skip to content

Make run-tests.php check for tcp fwrite edge cases - #8023

Closed
TysonAndre wants to merge 1 commit into
php:masterfrom
TysonAndre:run-tests-fwrite-error
Closed

Make run-tests.php check for tcp fwrite edge cases#8023
TysonAndre wants to merge 1 commit into
php:masterfrom
TysonAndre:run-tests-fwrite-error

Conversation

@TysonAndre

@TysonAndre TysonAndre commented Feb 3, 2022

Copy link
Copy Markdown
Contributor

When the recipient is busy or the payload is large, fwrite can block
or return a value smaller than the length of the stream.

workers in run-tests.php communicate over tcp sockets with the manager.

https://cirrus-ci.com/task/5315675320221696?logs=tests#L130
showed notices for fwrite/unserialize

This is a similar approach to that used in
https://github.com/phan/phan/blob/v5/src/Phan/LanguageServer/ProtocolStreamWriter.php
for the tcp language server writing.

Comment thread run-tests.php Outdated
When the recipient is busy or the payload is large, fwrite can block
or return a value smaller than the length of the stream.

workers in run-tests.php communicates over tcp sockets with the manager.

https://cirrus-ci.com/task/5315675320221696?logs=tests#L130
showed notices for fwrite/unserialize

This is a similar approach to that used in
https://github.com/phan/phan/blob/v5/src/Phan/LanguageServer/ProtocolStreamWriter.php
for the tcp language server writing.
Comment thread run-tests.php
$blocking = stream_get_meta_data($stream)["blocked"];
stream_set_blocking($stream, true);
fwrite($stream, base64_encode(serialize($message)) . "\n");
safe_fwrite($stream, base64_encode(serialize($message)) . "\n");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unused return value, should something happen if a write fails?

Comment thread run-tests.php
echo "ERROR: send_message() Failed to write chunk after 10 retries: " . error_get_last()['message'] . "\n";
return false;
}
$writeSockets = [$stream];

@iluuu1994 iluuu1994 Mar 24, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

$write_streams?

Comment thread run-tests.php
// safe_fwrite was tested by adding $message['unused'] = str_repeat('a', 20_000_000); in send_message()
// fwrites on tcp sockets can return false or less than strlen if the recipient is busy.
// (e.g. fwrite(): Send of 577 bytes failed with errno=35 Resource temporarily unavailable)
$bytesWritten = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The rest of this file uses snake_case

@github-actions

Copy link
Copy Markdown

There has not been any recent activity in this PR. It will automatically be closed in 7 days if no further action is taken.

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.

4 participants