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

PHP.wasm: yield 0 bytes read on fd_read failure to improve PHP's fread() and feof() behavior #1053

Merged
merged 3 commits into from Feb 26, 2024

Conversation

adamziel
Copy link
Collaborator

@adamziel adamziel commented Feb 26, 2024

Ensures that PHP.wasm indicates that feof($fp) === true when reading from pipes exposed by proc_open() after the process have exited and all the data have been read.

Internally in stdiop_read, PHP expects the read() syscall to return 0 while indicating that the number of bytes read was also 0. This PR makes the PHP.wasm polling logic comply with those expectations.

Testing instructions

Confirm the CI checks are green.

Ensures that PHP.wasm indicates that `feof($fp) === true` when reading
from pipes exposed by `proc_open()` after the process have exited and
all the data have been read.

Internally in stdiop_read, PHP expects the read() syscall to return 0
while indicating that the number of bytes read was also 0:

https://github.com/php/php-src/blob/718a8b4278cf811919906045d3bea4092e8a0844/main/streams/plain_wrapper.c#L426-L453

This PR makes the PHP.wasm polling logic comply with those expectations.

 ## Remaining work

Rebuild all the PHP versions.

 ## Testing instructions

Confirm the CI checks are green.
@adamziel adamziel changed the title PHP.wasm: Improve fread() and feof() behavior PHP.wasm: yield 0 bytes read on fd_read failure to improve PHP's fread() and feof() behavior Feb 26, 2024
@adamziel adamziel merged commit 97ca132 into trunk Feb 26, 2024
3 of 4 checks passed
@adamziel adamziel deleted the php-wasm-fix-feof-and-fread-from-finished-process branch February 26, 2024 15:19
var returnCode;
var stream;
let num = 0;
try {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh no, I've mixed tabs and spaces!

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

Successfully merging this pull request may close these issues.

None yet

1 participant