[PHP] Fix "Controller is already closed" crash in CLI server

packages/php-wasm/node/src/test/php-crash.spec.ts

@@ -132,6 +132,53 @@
 			}
 		});
 
+		it('Does not throw "Controller is already closed" when stream is cancelled before PHP exits', async () => {
+			// Reproduce the scenario where a stream consumer (e.g., the
+			// HTTP response pipeline in the CLI server) cancels the
+			// stdout stream before PHP finishes executing. Cancellation
+			// propagates to the controller, and then the finally block
+			// in #executeWithErrorHandling tries to close() the
+			// already-cancelled controller. Before the fix, this threw
+			// "Invalid state: Controller is already closed" and crashed
+			// the Node host process.
+			let resolveExecution!: (value: number) => void;
+			const spy = vi.spyOn(
+				php[__private__dont__use],
+				'ccall'
+			);
+			spy.mockImplementation((c_func) => {
+				if (c_func === 'wasm_sapi_handle_request') {
+					// Simulate PHP writing some output
+					php[__private__dont__use].onStdout?.(
+						new Uint8Array([72, 101, 108, 108, 111])
+					);
+					// Return a promise we control so we can cancel
+					// the stream before PHP "exits".
+					return new Promise((resolve) => {
+						resolveExecution = resolve;
+					});
+				}
+			});
+
+			const response = await php.runStream({
+				code: `<?php echo "test";`,
+			});
+
+			// Cancel the stdout stream — this simulates a client
+			// disconnect that destroys the HTTP response pipeline.
+			// The cancellation propagates to the controller.
+			await response.stdout.cancel('client disconnected');
+
+			// Now PHP "finishes". This triggers the finally block
+			// which calls controller.close() on the now-cancelled
+			// stdout controller. Before the fix, this threw.
+			resolveExecution(0);
+
+			// If the fix works, the exit code resolves normally.
+			const exitCode = await response.exitCode;
+			expect(exitCode).toBe(0);
+		});
+
 		it('Does not leak memory when creating and destroying instances', async () => {
 			if (!global.gc) {
 				console.error(

packages/php-wasm/universal/src/lib/api.ts

@@ -463,18 +463,31 @@ export function portToStream(port: MessagePort): ReadableStream<Uint8Array> {
 			const onMessage = (ev: MessageEvent) => {
 				const data: any = (ev as any).data;
 				if (!data) return;
-				switch (data.t) {
-					case 'chunk':
-						controller.enqueue(new Uint8Array(data.b));
-						break;
-					case 'close':
-						controller.close();
-						cleanup();
-						break;
-					case 'error':
-						controller.error(new Error(data.m || 'Stream error'));
-						cleanup();
-						break;
+				// Guard every controller operation with try-catch.
+				// The stream may have been cancelled by the consumer
+				// (e.g. HTTP client disconnect), which puts the
+				// controller in a closed state. Calling enqueue(),
+				// close(), or error() on a closed controller throws
+				// "Invalid state: Controller is already closed".
+				try {
+					switch (data.t) {
+						case 'chunk':
+							controller.enqueue(new Uint8Array(data.b));
+							break;
+						case 'close':
+							controller.close();
+							cleanup();
+							break;
+						case 'error':
+							controller.error(
+								new Error(data.m || 'Stream error')
+							);
+							cleanup();
+							break;
+					}
+				} catch {
+					// Stream already closed or errored — clean up.
+					cleanup();
 				}
 			};
 			const cleanup = () => {

packages/php-wasm/universal/src/lib/php.ts

@@ -1057,10 +1057,14 @@ export class PHP implements Disposable {
 					return e.status;
 				}
 
-				// Non-exit-code errors indicate a WASM runtime crash. Let's clean up and throw.
-				stdout.controller.error(e);
-				stderr.controller.error(e);
-				headers.controller.error(e);
+				// Non-exit-code errors indicate a WASM runtime crash.
+				// Let's clean up and throw. We use safeStreamError()
+				// because the headers controller may already be closed
+				// if onStdout fired before the crash (onStdout calls
+				// closeHeadersStream()).
+				safeStreamError(stdout.controller, e);
+				safeStreamError(stderr.controller, e);
+				safeStreamError(headers.controller, e);
 				streamsClosed = true;
 
 				/**
@@ -1083,8 +1087,11 @@ export class PHP implements Disposable {
 				throw e;
 			} finally {
 				if (!streamsClosed) {
-					stdout.controller.close();
-					stderr.controller.close();
+					// Close each stream individually so that a failure
+					// in one (e.g. stream cancelled by the consumer)
+					// doesn't prevent the others from being closed.
+					safeStreamClose(stdout.controller);
+					safeStreamClose(stderr.controller);
 					closeHeadersStream();
 					streamsClosed = true;
 				}
@@ -1592,15 +1599,23 @@ export class PHP implements Disposable {
 
 		const stderrStream = await createInvertedReadableStream<Uint8Array>();
 		process.on('error', (error) => {
-			stderrStream.controller.error(error);
+			safeStreamError(stderrStream.controller, error);
 		});
 		process.stderr.on('data', (data) => {
-			stderrStream.controller.enqueue(data);
+			try {
+				stderrStream.controller.enqueue(data);
+			} catch {
+				// Stream may have been cancelled by the consumer.
+			}
 		});
 
 		const stdoutStream = await createInvertedReadableStream<Uint8Array>();
 		process.stdout.on('data', (data) => {
-			stdoutStream.controller.enqueue(data);
+			try {
+				stdoutStream.controller.enqueue(data);
+			} catch {
+				// Stream may have been cancelled by the consumer.
+			}
 		});
 
 		process.on('exit', () => {
@@ -1764,6 +1779,37 @@ async function createInvertedReadableStream<T = BufferSource>(
 	};
 }
 
+/**
+ * Calls controller.error() but silently ignores the call if the
+ * controller's stream is already closed or errored. This happens
+ * when, e.g., onStdout closes the headers stream before a WASM
+ * crash propagates to the error-handling code.
+ */
+function safeStreamError(
+	controller: ReadableStreamDefaultController,
+	error: unknown
+) {
+	try {
+		controller.error(error);
+	} catch {
+		// The stream was already closed or errored – nothing to do.
+	}
+}
+
+/**
+ * Calls controller.close() but silently ignores the call if the
+ * controller's stream is already closed. This guards against a
+ * failure in one stream preventing sibling streams from being
+ * cleaned up.
+ */
+function safeStreamClose(controller: ReadableStreamDefaultController) {
+	try {
+		controller.close();
+	} catch {
+		// The stream was already closed or errored – nothing to do.
+	}
+}
+
 const getNodeType = (fs: Emscripten.FileSystemInstance, path: string) => {
 	try {
 		const target = fs.lookupPath(path, { follow: true });