Shell: Set pipes to non-blocking
authorBrad Jorsch <bjorsch@wikimedia.org>
Thu, 1 Feb 2018 18:45:35 +0000 (13:45 -0500)
committerBrad Jorsch <bjorsch@wikimedia.org>
Thu, 1 Feb 2018 21:04:12 +0000 (16:04 -0500)
The select(2) system call only guarantees a "sufficiently small write"
can be made without blocking. It doesn't define what that means.

And on Linux the read might block too in certain cases, although I don't
know if any of them can occur here.

Regardless, set all the pipes to non-blocking, which avoids the blocking
that's behind T184171.

And then, since a non-blocking read might validly return empty-string or
a non-blocking write might validly return 0, use feof() to check for EOF
and actually close the write pipe when it runs out of data.

Bug: T184171
Change-Id: I403235a328630112b6920905730f933777e2d453

includes/shell/Command.php

index 4f65e4d..d6f9578 100644 (file)
@@ -404,11 +404,20 @@ class Command {
                $eintr = defined( 'SOCKET_EINTR' ) ? SOCKET_EINTR : 4;
                $eintrMessage = "stream_select(): unable to select [$eintr]";
 
+               /* The select(2) system call only guarantees a "sufficiently small write"
+                * can be made without blocking. And on Linux the read might block too
+                * in certain cases, although I don't know if any of them can occur here.
+                * Regardless, set all the pipes to non-blocking to avoid T184171.
+                */
+               foreach ( $pipes as $pipe ) {
+                       stream_set_blocking( $pipe, false );
+               }
+
                $running = true;
                $timeout = null;
                $numReadyPipes = 0;
 
-               while ( $running === true || $numReadyPipes !== 0 ) {
+               while ( $pipes && ( $running === true || $numReadyPipes !== 0 ) ) {
                        if ( $running ) {
                                $status = proc_get_status( $proc );
                                // If the process has terminated, switch to nonblocking selects
@@ -465,14 +474,17 @@ class Command {
                                }
 
                                if ( $res === '' || $res === 0 ) {
-                                       // End of file
-                                       fclose( $pipes[$fd] );
-                                       unset( $pipes[$fd] );
-                                       if ( !$pipes ) {
-                                               break 2;
+                                       // End of file?
+                                       if ( feof( $pipe ) ) {
+                                               fclose( $pipes[$fd] );
+                                               unset( $pipes[$fd] );
                                        }
                                } elseif ( $isWrite ) {
-                                       $buffers[$fd] = substr( $buffers[$fd], $res );
+                                       $buffers[$fd] = (string)substr( $buffers[$fd], $res );
+                                       if ( $buffers[$fd] === '' ) {
+                                               fclose( $pipes[$fd] );
+                                               unset( $pipes[$fd] );
+                                       }
                                } else {
                                        $buffers[$fd] .= $res;
                                        if ( $fd === 3 && strpos( $res, "\n" ) !== false ) {