merged branch lenar/non-blocking-process (PR #1403)

Commits
-------

2d29a82 New test for Process, testing stdout and stderr at different stream sizes

Discussion
----------

Make run() fully non-blocking and fix potential other problems

Multiple changes:

1) make writing to process non-blocking too - otherwise there might be increased possibility for buffer deadlock
given big enough input data. Also now it's guaranteed that all stdin data will be written.

2) get rid of fgets() - fgets() isn't really good function to use in case of non-blocking sockets. Data loss possible.

---------------------------------------------------------------------------

by fabpot at 2011/06/22 07:11:55 -0700

Does it make https://github.com/symfony/symfony/pull/1365 obsolete?

---------------------------------------------------------------------------

by lenar at 2011/06/22 14:08:14 -0700

@fabpot: After reading, I really don't know. Let's hope. But ...

I now improved Process tests a bit to test stdout, stderr with different stream sizes and different
behaviours of child processes. Added it to non-blocking-process branch, commit 2d29a82412702faf5137.
In my case, nothing fails, but maybe this helps other people. Or Windows people - I myself cannot test on Windows.

---------------------------------------------------------------------------

by fabpot at 2011/06/22 22:59:55 -0700

These tests pass on my Linux box but fail on my Mac.

---------------------------------------------------------------------------

by fabpot at 2011/06/22 23:05:14 -0700

Actually, on the Mac, the tests behave correctly but the exit code is `-1` instead of `0`.

---------------------------------------------------------------------------

by lenar at 2011/06/23 01:23:51 -0700

Could you check if the $this->status['running'] (after call to proc_get_status()) is true in the case you get -1.

On my linux I got it -1 couple of times. 99% of time it doesn't happen. I theorized it's because sometimes the child
process isn't finished enough and finally I got confirmation too that in case of -1 the process is still running (stats['running'] === true).

But it's really almost unreproducible on my Linux. So if you have this value every time it might be easier for you to find solution.

What comes into my mind:

1) maybe we should poll, let's say if process is still running we usleep(1000) and the try proc_get_status() again until not running. Maybe up to a 1 sec.

2) maybe, if the process is still running we can trust the return value subsequently given by proc_close()?

Or maybe there's some other problem on Mac.

tests/Symfony/Tests/Component/Process/ProcessTest.php

@@ -38,24 +38,59 @@ class ProcessTest extends \PHPUnit_Framework_TestCase
     /**
      * tests results from sub processes
      *
-     * @dataProvider codeProvider
+     * @dataProvider responsesCodeProvider
      */
     public function testProcessResponses($expected, $getter, $code)
     {
-        $p = new Process(sprintf('php -r "%s"', $code));
+        $p = new Process(sprintf('php -r \'%s\'', $code));
         $p->run();
 
         $this->assertSame($expected, $p->$getter());
     }
 
-    public function codeProvider()
+    /**
+     * tests results from sub processes
+     *
+     * @dataProvider pipesCodeProvider
+     */
+    public function testProcessPipes($expected, $code)
+    {
+        $p = new Process(sprintf('php -r \'%s\'', $code));
+        $p->setStdin($expected);
+        $p->run();
+
+        $this->assertSame($expected, $p->getOutput());
+        $this->assertSame($expected, $p->getErrorOutput());
+        $this->assertSame(0, $p->getExitCode());
+    }
+
+    public function responsesCodeProvider()
     {
         return array(
             //expected output / getter / code to execute
             //array(1,'getExitCode','exit(1);'),
             //array(true,'isSuccessful','exit();'),
-            array('output', 'getOutput', 'echo \"output\";'),
+            array('output', 'getOutput', 'echo "output";'),
+        );
+    }
+     
+    public function pipesCodeProvider()
+    {
+        $variations = array(
+            'fwrite(STDOUT, $in = file_get_contents("php://stdin")); fwrite(STDERR, $in);',
+            'include "' . __DIR__ . '/ProcessTestHelper.php";',
         );
+        $baseData = str_repeat('*', 1024);
+
+        $codes = array();
+        foreach (array(1, 16, 64, 1024, 4096) as $size)
+        {
+            $data = str_repeat($baseData, $size) . '!';
+            foreach ($variations as $code) {
+                $codes[] = array($data, $code);
+            }
+        }
+        return $codes;
     }
 
     /**

tests/Symfony/Tests/Component/Process/ProcessTestHelper.php

@@ -0,0 +1,65 @@
+<?php
+
+define('ERR_SELECT_FAILED', 1);
+define('ERR_TIMEOUT', 2);
+define('ERR_READ_FAILED', 3);
+define('ERR_WRITE_FAILED', 4);
+
+$read = array(STDIN);
+$write = array(STDOUT, STDERR);
+
+stream_set_blocking(STDIN, false);
+stream_set_blocking(STDOUT, false);
+stream_set_blocking(STDERR, false);
+
+$out = $err = '';
+while ($read || $write) {
+    $r = $read;
+    $w = $write;
+    $e = null;
+    $n = stream_select($r, $w, $e, 5);
+    
+    if (false === $n) {
+        die(ERR_SELECT_FAILED);
+    } elseif ($n < 1) {
+        die(ERR_TIMEOUT);
+    }
+    
+    if (in_array(STDOUT, $w) && strlen($out) > 0)
+    {
+         $written = fwrite(STDOUT, (binary) $out, 1024);
+         if (false === $written) {
+             die(ERR_WRITE_FAILED);
+         }
+         $out = (binary) substr($out, $written);
+    }
+    if (null === $read && strlen($out) < 1) {
+        $write = array_diff($write, array(STDOUT));
+    }
+
+    if (in_array(STDERR, $w) && strlen($err) > 0)
+    {
+         $written = fwrite(STDERR, (binary) $err, 1024);
+         if (false === $written) {
+             die(ERR_WRITE_FAILED);
+         }
+         $err = (binary) substr($err, $written);
+    }
+    if (null === $read && strlen($err) < 1) {
+        $write = array_diff($write, array(STDERR));
+    }
+        
+    if ($r) {
+        $str = fread(STDIN, 1024);
+        if (false !== $str) {
+            $out .= $str;
+            $err .= $str;
+        } 
+        if (false === $str || feof(STDIN)) {
+            $read = null;
+            if (!feof(STDIN)) {
+                die(ERR_READ_FAILED);
+            }
+        }
+    }
+}