From 7974fe21c46cac2beea93e61213486e726303be5 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Tue, 29 Sep 2026 09:35:59 +0530 Subject: [PATCH 1/2] fix(process): read stdout and stderr together so large output can't deadlock Process::run() read stdout to EOF before it read stderr. A child that wrote more than a pipe buffer (about 64 KB) to stderr blocked on that write while EE waited for stdout, so both hung forever. EE::runcommand() with 'return' read its pipes the same way. The new Utils\read_pipes() waits on both pipes with stream_select() and reads whichever has data until both reach EOF. On Windows, where stream_select() can't wait on process pipes, or if stream_select() fails, it falls back to the previous blocking reads. --- php/EE/Process.php | 6 +++--- php/class-ee.php | 5 +++-- php/utils.php | 47 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/php/EE/Process.php b/php/EE/Process.php index 351d51e39..2990b5b0d 100644 --- a/php/EE/Process.php +++ b/php/EE/Process.php @@ -71,10 +71,10 @@ public function run() { $proc = Utils\proc_open_compat( $this->command, self::$descriptors, $pipes, $this->cwd, $this->env ); - $stdout = stream_get_contents( $pipes[1] ); + $output = Utils\read_pipes( $pipes ); + $stdout = $output[1]; + $stderr = $output[2]; fclose( $pipes[1] ); - - $stderr = stream_get_contents( $pipes[2] ); fclose( $pipes[2] ); $return_code = proc_close( $proc ); diff --git a/php/class-ee.php b/php/class-ee.php index e9dc1b0d3..e989654ca 100644 --- a/php/class-ee.php +++ b/php/class-ee.php @@ -1081,9 +1081,10 @@ public static function runcommand( $command, $options = array() ) { $proc = Utils\proc_open_compat( $runcommand, $descriptors, $pipes, getcwd() ); if ( $return ) { - $stdout = stream_get_contents( $pipes[1] ); + $output = Utils\read_pipes( $pipes ); + $stdout = $output[1]; + $stderr = $output[2]; fclose( $pipes[1] ); - $stderr = stream_get_contents( $pipes[2] ); fclose( $pipes[2] ); } $return_code = proc_close( $proc ); diff --git a/php/utils.php b/php/utils.php index 98052a3b9..239363ea5 100644 --- a/php/utils.php +++ b/php/utils.php @@ -1269,6 +1269,53 @@ function proc_open_compat( $cmd, $descriptorspec, &$pipes, $cwd = null, $env = n return proc_open( $cmd, $descriptorspec, $pipes, $cwd, $env, $other_options ); } +/** + * Read process pipes to EOF together, so a child can't block on a full pipe that isn't being read. + * + * @access public + * + * @param resource[] $pipes Pipes from `proc_open()`, keyed by descriptor number. + * + * @return string[] Contents of each pipe, with the same keys. + */ +function read_pipes( $pipes ) { + $output = array_fill_keys( array_keys( $pipes ), '' ); + $open = $pipes; + + // stream_select() can't wait on process pipes on Windows. + if ( ! is_windows() ) { + foreach ( $open as $pipe ) { + stream_set_blocking( $pipe, false ); + } + while ( $open ) { + $read = $open; + $write = null; + $except = null; + // @codingStandardsIgnoreLine + if ( false === @stream_select( $read, $write, $except, null ) ) { + break; + } + foreach ( $read as $key => $pipe ) { + $chunk = fread( $pipe, 65536 ); + if ( false !== $chunk ) { + $output[ $key ] .= $chunk; + } + if ( false === $chunk || feof( $pipe ) ) { + unset( $open[ $key ] ); + } + } + } + } + + // Blocking reads for whatever is left, as before. + foreach ( $open as $key => $pipe ) { + stream_set_blocking( $pipe, true ); + $output[ $key ] .= stream_get_contents( $pipe ); + } + + return $output; +} + /** * For use by `proc_open_compat()` only. Separated out for ease of testing. Windows only. * Turns *nix-like `ENV_VAR=blah command` environment variable prefixes into stripped `cmd` with prefixed environment variables added to passed in environment array. From fde7b3a2acee0981f928473b2ff928508c09d2e9 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Tue, 29 Sep 2026 09:48:02 +0530 Subject: [PATCH 2/2] fix(process): retry the pipe select when a signal interrupts it stream_select() returns false on EINTR, and select() is never restarted after a caught signal. site-command installs pcntl handlers for SIGTERM, SIGHUP, SIGUSR1 and SIGINT, so a signal during a site command dropped read_pipes() back to the sequential blocking reads, where more than 64 KB of stderr could hang again. Retry on an interrupted system call, as Symfony Process does, and fall back only on other errors. Also say in the docblock that read_pipes() takes output pipes only. --- php/utils.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/php/utils.php b/php/utils.php index 239363ea5..6260cbdad 100644 --- a/php/utils.php +++ b/php/utils.php @@ -1274,7 +1274,7 @@ function proc_open_compat( $cmd, $descriptorspec, &$pipes, $cwd = null, $env = n * * @access public * - * @param resource[] $pipes Pipes from `proc_open()`, keyed by descriptor number. + * @param resource[] $pipes Output (read) pipes from `proc_open()`, keyed by descriptor number. * * @return string[] Contents of each pipe, with the same keys. */ @@ -1291,8 +1291,14 @@ function read_pipes( $pipes ) { $read = $open; $write = null; $except = null; + error_clear_last(); // @codingStandardsIgnoreLine if ( false === @stream_select( $read, $write, $except, null ) ) { + // A caught signal (e.g. site-command's pcntl handlers) interrupts select(); retry instead of falling back. + $error = error_get_last(); + if ( $error && false !== stripos( $error['message'], 'interrupted system call' ) ) { + continue; + } break; } foreach ( $read as $key => $pipe ) {