bug #10455 [2.3][Process] Fix random failures in test suite on TravisCI (romainneutron)

This PR was merged into the 2.3 branch.

Discussion
----------

[2.3][Process] Fix random failures in test suite on TravisCI

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| License       | MIT

Commits
-------

783e377 [Process] Avoid failures because of slow IOs
238565e [Process] Avoid failure because of a slow process
173f8c5 [Process] Avoid failure when calling Process::stop in edge cases

src/Symfony/Component/Process/Process.php

@@ -356,17 +356,7 @@ class Process
      */
     public function signal($signal)
     {
-        if (!$this->isRunning()) {
-            throw new LogicException('Can not send signal on a non running process.');
-        }
-
-        if ($this->isSigchildEnabled()) {
-            throw new RuntimeException('This PHP has been compiled with --enable-sigchild. The process can not be signaled.');
-        }
-
-        if (true !== @proc_terminate($this->process, $signal)) {
-            throw new RuntimeException(sprintf('Error while sending signal `%d`.', $signal));
-        }
+        $this->doSignal($signal, true);
 
         return $this;
     }
@@ -635,7 +625,11 @@ class Process
 
             if ($this->isRunning() && !$this->isSigchildEnabled()) {
                 if (null !== $signal || defined('SIGKILL')) {
-                    $this->signal($signal ?: SIGKILL);
+                    // avoid exception here :
+                    // process is supposed to be running, but it might have stop
+                    // just after this line.
+                    // in any case, let's silently discard the error, we can not do anything
+                    $this->doSignal($signal ?: SIGKILL, false);
                 }
             }
         }
@@ -1111,4 +1105,44 @@ class Process
         $this->incrementalOutputOffset = 0;
         $this->incrementalErrorOutputOffset = 0;
     }
+
+    /**
+     * Sends a POSIX signal to the process.
+     *
+     * @param  integer $signal         A valid POSIX signal (see http://www.php.net/manual/en/pcntl.constants.php)
+     * @param  Boolean $throwException True to throw exception in case signal failed, false otherwise
+     * @return Boolean True if the signal was sent successfully, false otherwise
+     *
+     * @throws LogicException   In case the process is not running
+     * @throws RuntimeException In case --enable-sigchild is activated
+     * @throws RuntimeException In case of failure
+     */
+    private function doSignal($signal, $throwException)
+    {
+        if (!$this->isRunning()) {
+            if ($throwException) {
+                throw new LogicException('Can not send signal on a non running process.');
+            }
+
+            return false;
+        }
+
+        if ($this->isSigchildEnabled()) {
+            if ($throwException) {
+                throw new RuntimeException('This PHP has been compiled with --enable-sigchild. The process can not be signaled.');
+            }
+
+            return false;
+        }
+
+        if (true !== @proc_terminate($this->process, $signal)) {
+            if ($throwException) {
+                throw new RuntimeException(sprintf('Error while sending signal `%d`.', $signal));
+            }
+
+            return false;
+        }
+
+        return true;
+    }
 }

src/Symfony/Component/Process/ProcessPipes.php

@@ -29,6 +29,8 @@ class ProcessPipes
     /** @var Boolean */
     private $ttyMode;
 
+    const CHUNK_SIZE = 16384;
+
     public function __construct($useFiles, $ttyMode)
     {
         $this->useFiles = (Boolean) $useFiles;
@@ -233,7 +235,7 @@ class ProcessPipes
             $data = '';
             $dataread = null;
             while (!feof($fileHandle)) {
-                if (false !== $dataread = fread($fileHandle, 16392)) {
+                if (false !== $dataread = fread($fileHandle, self::CHUNK_SIZE)) {
                     $data .= $dataread;
                 }
             }
@@ -291,7 +293,7 @@ class ProcessPipes
             $type = array_search($pipe, $this->pipes);
 
             $data = '';
-            while ($dataread = fread($pipe, 8192)) {
+            while ($dataread = fread($pipe, self::CHUNK_SIZE)) {
                 $data .= $dataread;
             }
 

src/Symfony/Component/Process/Tests/AbstractProcessTest.php

@@ -13,6 +13,7 @@ namespace Symfony\Component\Process\Tests;
 
 use Symfony\Component\Process\Process;
 use Symfony\Component\Process\Exception\RuntimeException;
+use Symfony\Component\Process\ProcessPipes;
 
 /**
  * @author Robert Schönthal <seroscho@googlemail.com>
@@ -87,18 +88,20 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase
         // has terminated so the internal pipes array is already empty. normally
         // the call to start() will not read any data as the process will not have
         // generated output, but this is non-deterministic so we must count it as
-        // a possibility.  therefore we need 2 * 8192 plus another byte which will
-        // never be read.
-        $expectedOutputSize = 16385;
+        // a possibility.  therefore we need 2 * ProcessPipes::CHUNK_SIZE plus
+        // another byte which will never be read.
+        $expectedOutputSize = ProcessPipes::CHUNK_SIZE * 2 + 2;
 
         $code = sprintf('echo str_repeat(\'*\', %d);', $expectedOutputSize);
         $p = $this->getProcess(sprintf('php -r %s', escapeshellarg($code)));
 
         $p->start();
-        usleep(250000);
+        // Let's wait enough time for process to finish...
+        // Here we don't call Process::run or Process::wait to avoid any read of pipes
+        usleep(500000);
 
         if ($p->isRunning()) {
-            $this->fail('Process execution did not complete in the required time frame');
+            $this->markTestSkipped('Process execution did not complete in the required time frame');
         }
 
         $o = $p->getOutput();
@@ -200,7 +203,7 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase
 
     public function testGetIncrementalErrorOutput()
     {
-        $p = $this->getProcess(sprintf('php -r %s', escapeshellarg('$n = 0; while ($n < 3) { usleep(50000); file_put_contents(\'php://stderr\', \'ERROR\'); $n++; }')));
+        $p = $this->getProcess(sprintf('php -r %s', escapeshellarg('$n = 0; while ($n < 3) { usleep(100000); file_put_contents(\'php://stderr\', \'ERROR\'); $n++; }')));
 
         $p->start();
         while ($p->isRunning()) {
@@ -247,7 +250,7 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase
             $this->markTestSkipped('Windows does have /dev/tty support');
         }
 
-        $process = $this->getProcess('echo "foo" >> /dev/null');
+        $process = $this->getProcess('echo "foo" >> /dev/null && php -r "usleep(100000);"');
         $process->setTTY(true);
         $process->start();
         $this->assertTrue($process->isRunning());
@@ -468,7 +471,7 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase
         $process1->run();
         $process2 = $process1->restart();
 
-        usleep(300000); // wait for output
+        $process2->wait(); // wait for output
 
         // Ensure that both processed finished and the output is numeric
         $this->assertFalse($process1->isRunning());