From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Thu, 1 Feb 2018 18:45:35 +0000 (-0500)
Subject: Shell: Set pipes to non-blocking
X-Git-Tag: 1.31.0-rc.0~728^2
X-Git-Url: http://git.cyclocoop.org/%7B%24admin_url%7Dcompta/comptes/journal.php?a=commitdiff_plain;h=38bf4c15215ff3def72c3f45378939109e6649a5;p=lhc%2Fweb%2Fwiklou.git

Shell: Set pipes to non-blocking

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
---

diff --git a/includes/shell/Command.php b/includes/shell/Command.php
index 4f65e4d392..d6f95783f0 100644
--- a/includes/shell/Command.php
+++ b/includes/shell/Command.php
@@ -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 ) {