Windows fix

README.md

@@ -93,3 +93,40 @@ process in a chain ends, which would complicate working with I/O streams. As an
 alternative, considering launching one process at a time and listening on its
 `exit` event to conditionally start the next process in the chain. This will
 give you an opportunity to configure the subsequent process' I/O streams.
+
+### Windows compatibility
+
+Windows has always had a poor `proc_open` implementation in PHP. Even if things
+are better with the latest PHP versions, there are still a number of issues
+when programs are outputing values to STDOUT or STDERR (truncated output,
+deadlocks, ...).
+Prior to PHP 5.5.18 or PHP 5.6.3, a bug was simply causing a deadlock on
+Windows, making `proc_open` unusable.
+After PHP 5.5.18 and 5.6.3. Still, if the process
+you call outputs more than 4096 bytes, there are chances that your output will
+be truncated, or that your PHP process will stall for many seconds.
+
+Note: at the time of this writing, the bugs are still present in the current
+version of PHP, which arePHP 5.5.22 and PHP 5.6.6.
+
+To circumvent these problems, instead of relying on STDOUT
+or STDERR, *child-process* comes with a *Windows workaround* mode. You
+must activate it explicitly using the `useWindowsWorkaround` method.
+When activiated, it will redirect the output (STDOUT and STDERR) to files
+(in the temporary folder). This is an implementation detail but it is important
+to be aware of it. Indeed, the output of the command will be written to the
+disk, and even if the file is deleted at the end of the process, writing the
+output to the disk might be a security issues if the output contains sensitive
+data.
+It could also be an issue with long lasting commands that are outputing a lot
+of data since the output file will grow until the process ends. This might
+fill the hard-drive if the process lasts long enough.
+
+```php
+    $process = new React\ChildProcess\Process('echo foo');
+    $process->useWindowsWorkaround(true);
+    ...
+```
+
+Note: the `useWindowsWorkaround` is only used on Windows, it has no effect on
+other operating systems, so you can use it safely on Linux or MacOS.

src/Process.php

@@ -35,6 +35,8 @@ class Process extends EventEmitter
     private $stopSignal;
     private $termSignal;
 
+    private $windowsWorkaround = false;
+
     private static $sigchild;
 
     /**
@@ -82,6 +84,10 @@ public function start(LoopInterface $loop, $interval = 0.1)
             throw new \RuntimeException('Process is already running');
         }
 
+        if (defined('PHP_WINDOWS_VERSION_BUILD') && $this->windowsWorkaround) {
+            return $this->startWindows($loop, $interval);
+        }
+
         $cmd = $this->cmd;
         $fdSpec = array(
             array('pipe', 'r'), // stdin
@@ -119,6 +125,67 @@ public function start(LoopInterface $loop, $interval = 0.1)
         });
     }
 
+    /**
+     * This is a special implementation of the start method for the Windows operating system.
+     * This is needed because windows has a broken implementation of stdout / stderr pipes that
+     * cannot be fixed by PHP.
+     *
+     * @param LoopInterface $loop        Loop interface for stream construction
+     * @param float         $interval    Interval to periodically monitor process state (seconds)
+     * @throws RuntimeException If the process is already running or fails to start
+     */
+    protected function startWindows(LoopInterface $loop, $interval = 0.1)
+    {
+        $cmd = $this->cmd;
+
+        $stdoutName = tempnam(sys_get_temp_dir(), "out");
+        $stderrName = tempnam(sys_get_temp_dir(), "err");
+
+        // Let's open 2 file pointers for both stdout and stderr
+        // One for writing, one for reading.
+        $stdout = fopen($stdoutName, "w");
+        $stderr = fopen($stderrName, "w");
+        $stdoutRead = fopen($stdoutName, "r");
+        $stderrRead = fopen($stderrName, "r");
+
+
+        $fdSpec = array(
+            array('pipe', 'r'),
+            $stdout,
+            $stderr,
+        );
+
+        // Read exit code through fourth pipe to work around --enable-sigchild
+        if ($this->isSigchildEnabled() && $this->enhanceSigchildCompatibility) {
+            $fdSpec[] = array('pipe', 'w');
+            $cmd = sprintf('(%s) 3>/dev/null; code=$?; echo $code >&3; exit $code', $cmd);
+        }
+
+        $this->process = proc_open($cmd, $fdSpec, $this->pipes, $this->cwd, $this->env, $this->options);
+
+        if (!is_resource($this->process)) {
+            throw new \RuntimeException('Unable to launch a new process.');
+        }
+
+        $this->stdin  = new Stream($this->pipes[0], $loop);
+        $this->stdin->pause();
+        $this->stdout = new UnstopableStream($stdoutRead, $loop, $stdoutName);
+        $this->stderr = new UnstopableStream($stderrRead, $loop, $stderrName);
+
+        foreach ($this->pipes as $pipe) {
+            stream_set_blocking($pipe, 0);
+        }
+
+        $loop->addPeriodicTimer($interval, function (Timer $timer) use ($stdoutRead, $stderrRead) {
+            if (!$this->isRunning()) {
+                $this->close();
+                $timer->cancel();
+                $this->emit('exit', array($this->getExitCode(), $this->getTermSignal()));
+            }
+        });
+    }
+
+
     /**
      * Close the process.
      *
@@ -425,4 +492,12 @@ private function updateStatus()
             $this->exitCode = $this->status['exitcode'];
         }
     }
+
+    /**
+     * Sets whether the windows workaround mode should be used or not.
+     * @param bool $windowsWorkaround
+     */
+    public function useWindowsWorkaround($windowsWorkaround = true) {
+        $this->windowsWorkaround = $windowsWorkaround;
+    }
 }

src/UnstopableStream.php

@@ -0,0 +1,38 @@
+<?php
+namespace React\ChildProcess;
+
+use React\Stream\Stream;
+use React\EventLoop\LoopInterface;
+
+/**
+ * This is a special kind of stream that does not end when reaching EOF.
+ * Useful for the Windows workaround of STDOUT and STDERR due to buggy PHP implementation.
+ */
+class UnstopableStream extends Stream
+{
+    private $filename;
+
+    public function __construct($stream, LoopInterface $loop, $filename)
+    {
+        parent::__construct($stream, $loop);
+        $this->filename = $filename;    
+    }
+
+    public function handleData($stream)
+    {
+        $data = fread($stream, $this->bufferSize);
+
+        $this->emit('data', array($data, $this));
+
+        // Let's not stop on feof
+        if (!is_resource($stream)) {
+            $this->end();
+        }
+    }
+
+    public function handleClose()
+    {
+        parent::handleClose();
+        unlink($this->filename);
+    }
+}

tests/AbstractProcessTest.php

@@ -13,6 +13,7 @@ abstract public function createLoop();
     public function testGetEnhanceSigchildCompatibility()
     {
         $process = new Process('echo foo');
+        $process->useWindowsWorkaround();
 
         $this->assertSame($process, $process->setEnhanceSigchildCompatibility(true));
         $this->assertTrue($process->getEnhanceSigchildCompatibility());
@@ -27,6 +28,7 @@ public function testGetEnhanceSigchildCompatibility()
     public function testSetEnhanceSigchildCompatibilityCannotBeCalledIfProcessIsRunning()
     {
         $process = new Process('sleep 1');
+        $process->useWindowsWorkaround();
 
         $process->start($this->createLoop());
         $process->setEnhanceSigchildCompatibility(false);
@@ -35,13 +37,15 @@ public function testSetEnhanceSigchildCompatibilityCannotBeCalledIfProcessIsRunn
     public function testGetCommand()
     {
         $process = new Process('echo foo');
+        $process->useWindowsWorkaround();
 
         $this->assertSame('echo foo', $process->getCommand());
     }
 
     public function testIsRunning()
     {
         $process = new Process('sleep 1');
+        $process->useWindowsWorkaround();
 
         $this->assertFalse($process->isRunning());
         $process->start($this->createLoop());
@@ -68,22 +72,30 @@ public function testGetTermSignalWhenRunning($process)
 
     public function testProcessWithDefaultCwdAndEnv()
     {
-        $cmd = $this->getPhpBinary() . ' -r ' . escapeshellarg('echo getcwd(), PHP_EOL, count($_SERVER), PHP_EOL;');
+        $cmd = $this->getPhpCommandLine('echo getcwd(), PHP_EOL, count($_SERVER), PHP_EOL;');
 
         $loop = $this->createLoop();
-        $process = new Process($cmd);
+        $process = new Process($cmd, null, null, array("bypass_shell"=>true));
+        $process->useWindowsWorkaround();
 
         $output = '';
+        $error = '';
 
-        $loop->addTimer(0.001, function(Timer $timer) use ($process, &$output) {
+        $loop->addTimer(0.001, function(Timer $timer) use ($process, &$output, &$error) {
             $process->start($timer->getLoop());
-            $process->stdout->on('data', function () use (&$output) {
-                $output .= func_get_arg(0);
+            $process->stdout->on('data', function ($data) use (&$output) {
+                $output .= $data;
+            });
+            $process->stderr->on('data', function ($data) use (&$error) {
+                $error .= $data;
             });
         });
 
         $loop->run();
 
+        $this->assertEmpty($error);
+        $this->assertNotEmpty($output);
+
         list($cwd, $envCount) = explode(PHP_EOL, $output);
 
         /* Child process should inherit the same current working directory and
@@ -96,10 +108,17 @@ public function testProcessWithDefaultCwdAndEnv()
 
     public function testProcessWithCwd()
     {
-        $cmd = $this->getPhpBinary() . ' -r ' . escapeshellarg('echo getcwd(), PHP_EOL;');
+        $cmd = $this->getPhpCommandLine('echo getcwd(), PHP_EOL;');
+
+        if (defined('PHP_WINDOWS_VERSION_BUILD')) {
+            $testCwd = 'C:\\';
+        } else {
+            $testCwd = '/';
+        }
 
         $loop = $this->createLoop();
-        $process = new Process($cmd, '/');
+        $process = new Process($cmd, $testCwd, null, array("bypass_shell"=>true));
+        $process->useWindowsWorkaround();
 
         $output = '';
 
@@ -112,7 +131,7 @@ public function testProcessWithCwd()
 
         $loop->run();
 
-        $this->assertSame('/' . PHP_EOL, $output);
+        $this->assertSame($testCwd . PHP_EOL, $output);
     }
 
     public function testProcessWithEnv()
@@ -121,10 +140,17 @@ public function testProcessWithEnv()
             $this->markTestSkipped('Cannot execute PHP processes with custom environments on Travis CI.');
         }
 
-        $cmd = $this->getPhpBinary() . ' -r ' . escapeshellarg('echo getenv("foo"), PHP_EOL;');
+        $cmd = $this->getPhpCommandLine('echo getenv("foo"), PHP_EOL;');
+
+        if (defined('PHP_WINDOWS_VERSION_BUILD')) {
+            // Windows madness! escapeshellarg seems to completely remove double quotes in Windows!
+            // We need to use simple quotes in our PHP code!
+            $cmd = $this->getPhpCommandLine('echo getenv(\'foo\'), PHP_EOL;');
+        }
 
         $loop = $this->createLoop();
-        $process = new Process($cmd, null, array('foo' => 'bar'));
+        $process = new Process($cmd, null, array('foo' => 'bar'), array("bypass_shell"=>true));
+        $process->useWindowsWorkaround();
 
         $output = '';
 
@@ -144,6 +170,7 @@ public function testStartAndAllowProcessToExitSuccessfullyUsingEventLoop()
     {
         $loop = $this->createLoop();
         $process = new Process('exit 0');
+        $process->useWindowsWorkaround();
 
         $called = false;
         $exitCode = 'initial';
@@ -173,6 +200,10 @@ public function testStartAndAllowProcessToExitSuccessfullyUsingEventLoop()
 
     public function testStartInvalidProcess()
     {
+        if (defined('PHP_WINDOWS_VERSION_BUILD')) {
+            $this->markTestSkipped('Windows does not have an executable flag. This test does not make sense on Windows.');
+        }
+
         $cmd = tempnam(sys_get_temp_dir(), 'react');
 
         $loop = $this->createLoop();
@@ -200,6 +231,7 @@ public function testStartInvalidProcess()
     public function testStartAlreadyRunningProcess()
     {
         $process = new Process('sleep 1');
+        $process->useWindowsWorkaround();
 
         $process->start($this->createLoop());
         $process->start($this->createLoop());
@@ -257,6 +289,7 @@ public function testTerminateWithStopAndContinueSignalsUsingEventLoop()
 
         $loop = $this->createloop();
         $process = new Process('sleep 1; exit 0');
+        $process->useWindowsWorkaround();
 
         $called = false;
         $exitCode = 'initial';
@@ -298,6 +331,52 @@ public function testTerminateWithStopAndContinueSignalsUsingEventLoop()
         $this->assertFalse($process->isTerminated());
     }
 
+    public function outputSizeProvider() {
+        return [ [1000, 5], [10000, 5], [100000, 5] ];
+    }
+
+    /**
+     * @dataProvider outputSizeProvider
+     */
+    public function testProcessOutputOfSize($size, $expectedMaxDuration = 5)
+    {
+        // Note: very strange behaviour of Windows (PHP 5.5.6):
+        // on a 1000 long string, Windows succeeds.
+        // on a 10000 long string, Windows fails to output anything.
+        // On a 100000 long string, it takes a lot of time but succeeds.
+        $cmd = $this->getPhpBinary() . ' -r ' . escapeshellarg('echo str_repeat(\'o\', '.$size.'), PHP_EOL;');
+
+        if (defined('PHP_WINDOWS_VERSION_BUILD')) {
+            // Windows madness! for some obscure reason, the whole command lines needs to be
+            // wrapped in quotes (?!?)
+            $cmd = '"'.$cmd.'"';
+        }
+
+        $loop = $this->createLoop();
+        $process = new Process($cmd);
+        $process->useWindowsWorkaround();
+
+        $output = '';
+
+        $loop->addTimer(0.001, function(Timer $timer) use ($process, &$output) {
+            $process->start($timer->getLoop());
+            $process->stdout->on('data', function () use (&$output) {
+                $output .= func_get_arg(0);
+            });
+        });
+
+        $startTime = time();
+
+        $loop->run();
+
+        $endTime = time();
+
+        $this->assertEquals($size + strlen(PHP_EOL), strlen($output));
+        $this->assertSame(str_repeat('o', $size) . PHP_EOL, $output);
+        $this->assertLessThanOrEqual($expectedMaxDuration, $endTime - $startTime, "Process took longer than expected.");
+    }
+
+
     /**
      * Execute a callback at regular intervals until it returns successfully or
      * a timeout is reached.
@@ -333,4 +412,9 @@ private function getPhpBinary()
 
         return $runtime->getBinary();
     }
+
+    private function getPhpCommandLine($phpCode)
+    {
+        return $this->getPhpBinary() . ' -r ' . escapeshellarg($phpCode);
+    }
 }