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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 79 additions & 62 deletions packages/php-wasm/compile/php/phpwasm-emscripten-library.js
Original file line number Diff line number Diff line change
Expand Up @@ -722,74 +722,91 @@ const LibraryExample = {
* @see https://github.com/emscripten-core/emscripten/issues/13214
*/
js_fd_read: function (fd, iov, iovcnt, pnum) {
var returnCode;
var stream;
try {
stream = SYSCALLS.getStreamFromFD(fd);
var num = doReadv(stream, iov, iovcnt);
HEAPU32[pnum >> 2] = num;
returnCode = 0;
} catch (e) {
if (typeof FS == 'undefined' || !(e.name === 'ErrnoError')) throw e;
returnCode = e.errno;
// Only run the read operation on a regular call,
// never when rewinding the stack.
if (Asyncify.state === Asyncify.State.Normal) {
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!

stream = SYSCALLS.getStreamFromFD(fd);
const num = doReadv(stream, iov, iovcnt);
HEAPU32[pnum >> 2] = num;
return 0;
} catch (e) {
// Rethrow any unexpected non-filesystem errors.
if (typeof FS == "undefined" || !(e.name === "ErrnoError")) {
throw e;
}
// Only return synchronously if this isn't an asynchronous pipe.
// Error code 6 indicates EWOULDBLOCK – this is our signal to wait.
// We also need to distinguish between a process pipe and a file pipe, otherwise
// reading from an empty file would block until the timeout.
if (e.errno !== 6 || !(stream?.fd in PHPWASM.child_proc_by_fd)) {
// On failure, yield 0 bytes read to indicate EOF.
HEAPU32[pnum >> 2] = 0;
return returnCode
}
}
}

// If it's a blocking process pipe, wait for it to become readable.
// We need to distinguish between a process pipe and a file pipe, otherwise
// reading from an empty file would block until the timeout.
if (
returnCode === 6 /*EWOULDBLOCK*/ &&
stream?.fd in PHPWASM.child_proc_by_fd
) {
// You might wonder why we duplicate the code here instead of always using
// Asyncify.handleSleep(). The reason is performance. Most of the time,
// the read operation will work synchronously and won't require yielding
// back to JS. In these cases we don't want to pay the Asyncify overhead,
// save the stack, yield back to JS, restore the stack etc.
return Asyncify.handleSleep(function (wakeUp) {
var retries = 0;
var interval = 50;
var timeout = 5000;
// We poll for data and give up after a timeout.
// We can't simply rely on PHP timeout here because we don't want
// to, say, block the entire PHPUnit test suite without any visible
// feedback.
var maxRetries = timeout / interval;
function poll() {
var returnCode;
var stream;
try {
stream = SYSCALLS.getStreamFromFD(fd);
var num = doReadv(stream, iov, iovcnt);
HEAPU32[pnum >> 2] = num;
returnCode = 0;
} catch (e) {
if (
typeof FS == 'undefined' ||
!(e.name === 'ErrnoError')
) {
console.error(e);
throw e;
}
returnCode = e.errno;
}

// At this point we know we have to poll.
// You might wonder why we duplicate the code here instead of always using
// Asyncify.handleSleep(). The reason is performance. Most of the time,
// the read operation will work synchronously and won't require yielding
// back to JS. In these cases we don't want to pay the Asyncify overhead,
// save the stack, yield back to JS, restore the stack etc.
return Asyncify.handleSleep(function (wakeUp) {
var retries = 0;
var interval = 50;
var timeout = 5000;
// We poll for data and give up after a timeout.
// We can't simply rely on PHP timeout here because we don't want
// to, say, block the entire PHPUnit test suite without any visible
// feedback.
var maxRetries = timeout / interval;
function poll() {
var returnCode;
var stream;
let num;
try {
stream = SYSCALLS.getStreamFromFD(fd);
num = doReadv(stream, iov, iovcnt);
returnCode = 0;
} catch (e) {
if (
returnCode !== 6 ||
++retries > maxRetries ||
!(fd in PHPWASM.child_proc_by_fd) ||
PHPWASM.child_proc_by_fd[fd]?.exited ||
FS.isClosed(stream)
typeof FS == 'undefined' ||
!(e.name === 'ErrnoError')
) {
wakeUp(returnCode);
} else {
setTimeout(poll, interval);
console.error(e);
throw e;
}
returnCode = e.errno;
}
poll();
});
}
return returnCode;

const success = returnCode === 0;
const failure = (
++retries > maxRetries ||
!(fd in PHPWASM.child_proc_by_fd) ||
PHPWASM.child_proc_by_fd[fd]?.exited ||
FS.isClosed(stream)
);

if (success) {
HEAPU32[pnum >> 2] = num;
wakeUp(0);
} else if (failure) {
// On failure, yield 0 bytes read to indicate EOF.
HEAPU32[pnum >> 2] = 0;
// If the failure is due to a timeout, return 0 to indicate that we
// reached EOF. Otherwise, propagate the error code.
wakeUp(returnCode === 6 ? 0 : returnCode);
} else {
setTimeout(poll, interval);
}
}
poll();
});
},

/**
Expand Down
Loading
Loading