Skip to content

Commit 62ae031

Browse files
authored
[PHP] Fix "Controller is already closed" crash in CLI server (#3441)
## Summary This PR fixes two related Node-side crash paths in PHP-WASM that could bring down the Playground CLI server instead of failing a single request cleanly. The first crash happened when PHP had already produced output, which closes the headers stream, and then execution failed afterwards. In that case, Node could still try to `error()` a `ReadableStream` controller that had already been closed, which threw: ```text TypeError [ERR_INVALID_STATE]: Invalid state: Controller is already closed ``` The validated repro for that path is: 1. PHP writes output 2. `closeHeadersStream()` closes the headers controller 3. a later WASM/runtime error occurs 4. error handling tries to signal the already-closed controller A simple `echo ...; exit;` by itself is not the exact repro. The second crash happened when a stream consumer cancelled the response stream before PHP finished, for example because the HTTP pipeline completed or the client disconnected. In that case, the runtime could still try to `close()` an already-cancelled controller and hit the same `Controller is already closed` failure. The third crash path was in the `proc_open()` bridge used by the Node PHP runtime. If a spawned process timed out or exited while Node still had buffered stdout/stderr chunks to forward, the generated `php.js` bridge could try to write into a PIPEFS stream that PHP had already closed. That crashed the host with an error like: ```text TypeError: Cannot read properties of null (reading 'length') ``` Example: ```php <?php $res = proc_open( "hanging_command", [ ["pipe", "r"], ["pipe", "w"], ["pipe", "w"], ], $pipes ); fread($pipes[1], 1024); ``` ## What changed - Added defensive stream-controller handling in the universal runtime and stream bridge so already-closed controllers no longer crash the host process. - Fixed child stdout/stderr teardown in the Emscripten Node bridge by detaching listeners on exit and ignoring late chunks after PHP has already closed the child-side pipe. - Added `off()` to the shared `EventEmitter` interface so the child-process typing matches the APIs used by the runtime cleanup logic. - Regenerated all checked-in Node PHP binaries (`asyncify` and `jspi`, PHP 7.4 through 8.5) from the updated build source. ## Test plan CI
1 parent 66aedcd commit 62ae031

File tree

35 files changed

+144049
-170783
lines changed

35 files changed

+144049
-170783
lines changed

packages/php-wasm/compile/php/phpwasm-emscripten-library.js

Lines changed: 43 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -661,8 +661,12 @@ const LibraryExample = {
661661
stdoutParentFd = std[1]?.parent,
662662
stderrChildFd = std[2]?.child,
663663
stderrParentFd = std[2]?.parent;
664+
const detachPipeDataListeners = [];
664665

665666
cp.on('exit', function (code) {
667+
for (const detach of detachPipeDataListeners) {
668+
detach();
669+
}
666670
for (const fd of [
667671
// The child process exited. Let's clean up its output streams:
668672
stdoutChildFd,
@@ -689,16 +693,27 @@ const LibraryExample = {
689693
stdoutChildFd
690694
);
691695
let stdoutAt = 0;
692-
cp.stdout.on('data', function (data) {
693-
stdoutStream.stream_ops.write(
694-
stdoutStream,
695-
data,
696-
0,
697-
data.length,
698-
stdoutAt
699-
);
700-
stdoutAt += data.length;
701-
});
696+
const onStdoutData = function (data) {
697+
try {
698+
stdoutStream.stream_ops.write(
699+
stdoutStream,
700+
data,
701+
0,
702+
data.length,
703+
stdoutAt
704+
);
705+
stdoutAt += data.length;
706+
} catch {
707+
// PHP may close the child pipe before Node finishes
708+
// draining already-buffered stdout data. Late chunks are
709+
// no longer deliverable, so detach the listener and stop.
710+
cp.stdout.off('data', onStdoutData);
711+
}
712+
};
713+
cp.stdout.on('data', onStdoutData);
714+
detachPipeDataListeners.push(() =>
715+
cp.stdout.off('data', onStdoutData)
716+
);
702717
}
703718

704719
// Pass data from child process's stderr to PHP's end of the stdout pipe.
@@ -707,16 +722,24 @@ const LibraryExample = {
707722
stderrChildFd
708723
);
709724
let stderrAt = 0;
710-
cp.stderr.on('data', function (data) {
711-
stderrStream.stream_ops.write(
712-
stderrStream,
713-
data,
714-
0,
715-
data.length,
716-
stderrAt
717-
);
718-
stderrAt += data.length;
719-
});
725+
const onStderrData = function (data) {
726+
try {
727+
stderrStream.stream_ops.write(
728+
stderrStream,
729+
data,
730+
0,
731+
data.length,
732+
stderrAt
733+
);
734+
stderrAt += data.length;
735+
} catch {
736+
cp.stderr.off('data', onStderrData);
737+
}
738+
};
739+
cp.stderr.on('data', onStderrData);
740+
detachPipeDataListeners.push(() =>
741+
cp.stderr.off('data', onStderrData)
742+
);
720743
}
721744

722745
/**
39 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)