merged branch romainneutron/FixProcessStop (PR #5543)

Commits
-------

7bafc69 Add a Sigchild compatibility mode (set to false by default)

Discussion
----------

[2.1][Process] Fix stop in non-sigchild environments

Bug fix: yes
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT

Fix #5030 in half way.

 - `proc_terminate` now sends the `SIGTERM` to the real process, not the sh (add the exec prefix to remove the wrapper as suggested by @schmittjoh). So now, process will stop, except if you're working with a PHP compiled with `--enable-sigchild`
 - Add a Sigchild compatibility mode (activated only if PHP has been compiled with it)

This mode is required to use a hack to determine if the process finished with success when PHP has been compiled with the --enable-sigchild option

#5030 will be totally fixed in 2.2 with #5476 as it would allow to send a `SIGKILL` after timeout

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

by stof at 2012-09-18T21:19:50Z

This will also fix the error reported in Behat/MinkZombieDriver#10
The stop method was broken because of the sigchild workaround introduced in the latest 2.1-RC times

src/Symfony/Component/Process/Process.php

@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\Process;
 
+use Symfony\Component\Process\Exception\RuntimeException;
+
 /**
  * Process is a thin wrapper around proc_* functions to ease
  * start independent PHP processes.
@@ -44,6 +46,7 @@ class Process
     private $stdout;
     private $stderr;
     private $enhanceWindowsCompatibility;
+    private $enhanceSigchildCompatibility;
     private $pipes;
     private $process;
     private $status = self::STATUS_READY;
@@ -51,6 +54,8 @@ class Process
     private $fileHandles;
     private $readBytes;
 
+    private static $sigchild;
+
     /**
      * Exit codes translation table.
      *
@@ -134,6 +139,7 @@ class Process
         $this->stdin = $stdin;
         $this->setTimeout($timeout);
         $this->enhanceWindowsCompatibility = true;
+        $this->enhanceSigchildCompatibility = !defined('PHP_WINDOWS_VERSION_BUILD') && $this->isSigchildEnabled();
         $this->options = array_replace(array('suppress_errors' => true, 'binary_pipes' => true), $options);
     }
 
@@ -216,9 +222,16 @@ class Process
                 array('pipe', 'r'), // stdin
                 array('pipe', 'w'), // stdout
                 array('pipe', 'w'), // stderr
-                array('pipe', 'w')  // last exit code is output on the fourth pipe and caught to work around --enable-sigchild
             );
-            $this->commandline = '('.$this->commandline.') 3>/dev/null; code=$?; echo $code >&3; exit $code';
+
+            if ($this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) {
+                // last exit code is output on the fourth pipe and caught to work around --enable-sigchild
+                $descriptors = array_merge($descriptors, array(array('pipe', 'w')));
+
+                $this->commandline = '('.$this->commandline.') 3>/dev/null; code=$?; echo $code >&3; exit $code';
+            } else {
+                $this->commandline = 'exec ' . $this->commandline;
+            }
         }
 
         $commandline = $this->commandline;
@@ -418,10 +431,16 @@ class Process
      *
      * @return integer The exit status code
      *
+     * @throws RuntimeException In case --enable-sigchild is activated and the sigchild compatibility mode is disabled
+     *
      * @api
      */
     public function getExitCode()
     {
+        if ($this->isSigchildEnabled() && !$this->enhanceSigchildCompatibility) {
+            throw new RuntimeException('This PHP has been compiled with --enable-sigchild. You must use setEnhanceSigchildCompatibility() to use this method');
+        }
+
         $this->updateStatus();
 
         return $this->exitcode;
@@ -435,14 +454,16 @@ class Process
      *
      * @return string A string representation for the exit status code
      *
+     * @throws RuntimeException In case --enable-sigchild is activated and the sigchild compatibility mode is disabled
+     *
      * @see http://tldp.org/LDP/abs/html/exitcodes.html
      * @see http://en.wikipedia.org/wiki/Unix_signal
      */
     public function getExitCodeText()
     {
-        $this->updateStatus();
+        $exitcode = $this->getExitCode();
 
-        return isset(self::$exitCodes[$this->exitcode]) ? self::$exitCodes[$this->exitcode] : 'Unknown error';
+        return isset(self::$exitCodes[$exitcode]) ? self::$exitCodes[$exitcode] : 'Unknown error';
     }
 
     /**
@@ -450,13 +471,13 @@ class Process
      *
      * @return Boolean true if the process ended successfully, false otherwise
      *
+     * @throws RuntimeException In case --enable-sigchild is activated and the sigchild compatibility mode is disabled
+     *
      * @api
      */
     public function isSuccessful()
     {
-        $this->updateStatus();
-
-        return 0 == $this->exitcode;
+        return 0 == $this->getExitCode();
     }
 
     /**
@@ -466,10 +487,16 @@ class Process
      *
      * @return Boolean
      *
+     * @throws RuntimeException In case --enable-sigchild is activated
+     *
      * @api
      */
     public function hasBeenSignaled()
     {
+        if ($this->isSigchildEnabled()) {
+            throw new RuntimeException('This PHP has been compiled with --enable-sigchild. Term signal can not be retrieved');
+        }
+
         $this->updateStatus();
 
         return $this->processInformation['signaled'];
@@ -482,10 +509,16 @@ class Process
      *
      * @return integer
      *
+     * @throws RuntimeException In case --enable-sigchild is activated
+     *
      * @api
      */
     public function getTermSignal()
     {
+        if ($this->isSigchildEnabled()) {
+            throw new RuntimeException('This PHP has been compiled with --enable-sigchild. Term signal can not be retrieved');
+        }
+
         $this->updateStatus();
 
         return $this->processInformation['termsig'];
@@ -678,6 +711,30 @@ class Process
         $this->enhanceWindowsCompatibility = (Boolean) $enhance;
     }
 
+    /**
+     * Return whether sigchild compatibility mode is activated or not
+     *
+     * @return Boolean
+     */
+    public function getEnhanceSigchildCompatibility()
+    {
+        return $this->enhanceSigchildCompatibility;
+    }
+
+    /**
+     * Activate sigchild compatibility mode
+     *
+     * Sigchild compatibility mode is required to get the exit code and
+     * determine the success of a process when PHP has been compiled with
+     * the --enable-sigchild option
+     *
+     * @param Boolean $enhance
+     */
+    public function setEnhanceSigchildCompatibility($enhance)
+    {
+        $this->enhanceSigchildCompatibility = (Boolean) $enhance;
+    }
+
     /**
      * Builds up the callback used by wait().
      *
@@ -743,6 +800,23 @@ class Process
         }
     }
 
+    /**
+     * Return whether PHP has been compiled with the '--enable-sigchild' option or not
+     *
+     * @return Boolean
+     */
+    protected function isSigchildEnabled()
+    {
+        if (null !== self::$sigchild) {
+            return self::$sigchild;
+        }
+
+        ob_start();
+        phpinfo(INFO_GENERAL);
+
+        return self::$sigchild = false !== strpos(ob_get_clean(), '--enable-sigchild');
+    }
+
     /**
      * Handles the windows file handles fallbacks
      *

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

@@ -11,19 +11,19 @@
 
 namespace Symfony\Component\Process\Tests;
 
-use Symfony\Component\Process\Process;
-
 /**
  * @author Robert Schönthal <seroscho@googlemail.com>
  */
-class ProcessTest extends \PHPUnit_Framework_TestCase
+abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase
 {
+    protected abstract function getProcess($commandline, $cwd = null, array $env = null, $stdin = null, $timeout = 60, array $options = array());
+
     /**
      * @expectedException \InvalidArgumentException
      */
     public function testNegativeTimeoutFromConstructor()
     {
-        new Process('', null, null, null, -1);
+        $this->getProcess('', null, null, null, -1);
     }
 
     /**
@@ -31,13 +31,13 @@ class ProcessTest extends \PHPUnit_Framework_TestCase
      */
     public function testNegativeTimeoutFromSetter()
     {
-        $p = new Process('');
+        $p = $this->getProcess('');
         $p->setTimeout(-1);
     }
 
     public function testNullTimeout()
     {
-        $p = new Process('');
+        $p = $this->getProcess('');
         $p->setTimeout(10);
         $p->setTimeout(null);
 
@@ -51,7 +51,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase
      */
     public function testProcessResponses($expected, $getter, $code)
     {
-        $p = new Process(sprintf('php -r %s', escapeshellarg($code)));
+        $p = $this->getProcess(sprintf('php -r %s', escapeshellarg($code)));
         $p->run();
 
         $this->assertSame($expected, $p->$getter());
@@ -64,22 +64,21 @@ class ProcessTest extends \PHPUnit_Framework_TestCase
      */
     public function testProcessPipes($expected, $code)
     {
-        if (strpos(PHP_OS, "WIN") === 0) {
+        if (defined('PHP_WINDOWS_VERSION_BUILD')) {
             $this->markTestSkipped('Test hangs on Windows & PHP due to https://bugs.php.net/bug.php?id=60120 and https://bugs.php.net/bug.php?id=51800');
         }
 
-        $p = new Process(sprintf('php -r %s', escapeshellarg($code)));
+        $p = $this->getProcess(sprintf('php -r %s', escapeshellarg($code)));
         $p->setStdin($expected);
         $p->run();
 
         $this->assertSame($expected, $p->getOutput());
         $this->assertSame($expected, $p->getErrorOutput());
-        $this->assertSame(0, $p->getExitCode());
     }
 
     public function testCallbackIsExecutedForOutput()
     {
-        $p = new Process(sprintf('php -r %s', escapeshellarg('echo \'foo\';')));
+        $p = $this->getProcess(sprintf('php -r %s', escapeshellarg('echo \'foo\';')));
 
         $called = false;
         $p->run(function ($type, $buffer) use (&$called) {
@@ -91,12 +90,12 @@ class ProcessTest extends \PHPUnit_Framework_TestCase
 
     public function testExitCodeCommandFailed()
     {
-        if (strpos(PHP_OS, "WIN") === 0) {
+        if (defined('PHP_WINDOWS_VERSION_BUILD')) {
             $this->markTestSkipped('Windows does not support POSIX exit code');
         }
 
         // such command run in bash return an exitcode 127
-        $process = new Process('nonexistingcommandIhopeneversomeonewouldnameacommandlikethis');
+        $process = $this->getProcess('nonexistingcommandIhopeneversomeonewouldnameacommandlikethis');
         $process->run();
 
         $this->assertGreaterThan(0, $process->getExitCode());
@@ -104,7 +103,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase
 
     public function testExitCodeText()
     {
-        $process = new Process('');
+        $process = $this->getProcess('');
         $r = new \ReflectionObject($process);
         $p = $r->getProperty('exitcode');
         $p->setAccessible(true);
@@ -115,7 +114,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase
 
     public function testStartIsNonBlocking()
     {
-        $process = new Process('php -r "sleep(4);"');
+        $process = $this->getProcess('php -r "sleep(4);"');
         $start = microtime(true);
         $process->start();
         $end = microtime(true);
@@ -124,16 +123,21 @@ class ProcessTest extends \PHPUnit_Framework_TestCase
 
     public function testUpdateStatus()
     {
-        $process = new Process('php -h');
-        $process->start();
-        usleep(300000); // wait for output
-        $this->assertEquals(0, $process->getExitCode());
+        $process = $this->getProcess('php -h');
+        $process->run();
         $this->assertTrue(strlen($process->getOutput()) > 0);
     }
 
+    public function testGetExitCode()
+    {
+        $process = $this->getProcess('php -m');
+        $process->run();
+        $this->assertEquals(0, $process->getExitCode());
+    }
+
     public function testIsRunning()
     {
-        $process = new Process('php -r "sleep(1);"');
+        $process = $this->getProcess('php -r "sleep(1);"');
         $this->assertFalse($process->isRunning());
         $process->start();
         $this->assertTrue($process->isRunning());
@@ -143,16 +147,74 @@ class ProcessTest extends \PHPUnit_Framework_TestCase
 
     public function testStop()
     {
-        $process = new Process('php -r "while (true) {}"');
+        $process = $this->getProcess('php -r "while (true) {}"');
         $process->start();
         $this->assertTrue($process->isRunning());
         $process->stop();
         $this->assertFalse($process->isRunning());
+    }
 
-        // skip this check on windows since it does not support signals
-        if (!defined('PHP_WINDOWS_VERSION_MAJOR')) {
-            $this->assertTrue($process->hasBeenSignaled());
+    public function testIsSuccessful()
+    {
+        $process = $this->getProcess('php -m');
+        $process->run();
+        $this->assertTrue($process->isSuccessful());
+    }
+
+    public function testIsNotSuccessful()
+    {
+        $process = $this->getProcess('php -r "while (true) {}"');
+        $process->start();
+        $this->assertTrue($process->isRunning());
+        $process->stop();
+        $this->assertFalse($process->isSuccessful());
+    }
+
+    public function testProcessIsNotSignaled()
+    {
+        if (defined('PHP_WINDOWS_VERSION_BUILD')) {
+            $this->markTestSkipped('Windows does not support POSIX signals');
         }
+
+        $process = $this->getProcess('php -m');
+        $process->run();
+        $this->assertFalse($process->hasBeenSignaled());
+    }
+
+    public function testProcessWithoutTermSignal()
+    {
+        if (defined('PHP_WINDOWS_VERSION_BUILD')) {
+            $this->markTestSkipped('Windows does not support POSIX signals');
+        }
+
+        $process = $this->getProcess('php -m');
+        $process->run();
+        $this->assertEquals(0, $process->getTermSignal());
+    }
+
+    public function testProcessIsSignaledIfStopped()
+    {
+        if (defined('PHP_WINDOWS_VERSION_BUILD')) {
+            $this->markTestSkipped('Windows does not support POSIX signals');
+        }
+
+        $process = $this->getProcess('php -r "while (true) {}"');
+        $process->start();
+        $process->stop();
+        $this->assertTrue($process->hasBeenSignaled());
+    }
+
+    public function testProcessWithTermSignal()
+    {
+        if (defined('PHP_WINDOWS_VERSION_BUILD')) {
+            $this->markTestSkipped('Windows does not support POSIX signals');
+        }
+
+
+        $process = $this->getProcess('php -r "while (true) {}"');
+        $process->start();
+        $process->stop();
+        $this->assertEquals(SIGTERM, $process->getTermSignal());
     }
 
     public function testPhpDeadlock()
@@ -161,7 +223,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase
 
         // Sleep doesn't work as it will allow the process to handle signals and close
         // file handles from the other end.
-        $process = new Process('php -r "while (true) {}"');
+        $process = $this->getProcess('php -r "while (true) {}"');
         $process->start();
 
         // PHP will deadlock when it tries to cleanup $process

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

@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Process\Tests;
+
+use Symfony\Component\Process\Process;
+
+class ProcessInSigchildEnvironment extends Process
+{
+    protected function isSigchildEnabled()
+    {
+        return true;
+    }
+}

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

@@ -0,0 +1,99 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Process\Tests;
+
+class SigchildDisabledProcessTest extends AbstractProcessTest
+{
+
+    protected function getProcess($commandline, $cwd = null, array $env = null, $stdin = null, $timeout = 60, array $options = array())
+    {
+        $process = new ProcessInSigchildEnvironment($commandline, $cwd, $env, $stdin, $timeout, $options);
+        $process->setEnhanceSigchildCompatibility(false);
+
+        return $process;
+    }
+
+    /**
+     * @expectedException Symfony\Component\Process\Exception\RuntimeException
+     */
+    public function testGetExitCode()
+    {
+        parent::testGetExitCode();
+    }
+
+    /**
+     * @expectedException Symfony\Component\Process\Exception\RuntimeException
+     */
+    public function testExitCodeCommandFailed()
+    {
+        parent::testExitCodeCommandFailed();
+    }
+
+    /**
+     * @expectedException Symfony\Component\Process\Exception\RuntimeException
+     */
+    public function testProcessIsSignaledIfStopped()
+    {
+        parent::testProcessIsSignaledIfStopped();
+    }
+
+    /**
+     * @expectedException Symfony\Component\Process\Exception\RuntimeException
+     */
+    public function testProcessWithTermSignal()
+    {
+        parent::testProcessWithTermSignal();
+    }
+
+    /**
+     * @expectedException Symfony\Component\Process\Exception\RuntimeException
+     */
+    public function testProcessIsNotSignaled()
+    {
+        parent::testProcessIsNotSignaled();
+    }
+
+    /**
+     * @expectedException Symfony\Component\Process\Exception\RuntimeException
+     */
+    public function testProcessWithoutTermSignal()
+    {
+        parent::testProcessWithoutTermSignal();
+    }
+
+    /**
+     * @expectedException Symfony\Component\Process\Exception\RuntimeException
+     */
+    public function testExitCodeText()
+    {
+        $process = $this->getProcess('qdfsmfkqsdfmqmsd');
+        $process->run();
+
+        $process->getExitCodeText();
+    }
+
+    /**
+     * @expectedException Symfony\Component\Process\Exception\RuntimeException
+     */
+    public function testIsSuccessful()
+    {
+        parent::testIsSuccessful();
+    }
+
+    /**
+     * @expectedException Symfony\Component\Process\Exception\RuntimeException
+     */
+    public function testIsNotSuccessful()
+    {
+        parent::testIsNotSuccessful();
+    }
+}

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

@@ -0,0 +1,65 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Process\Tests;
+
+class SigchildEnabledProcessTest extends AbstractProcessTest
+{
+
+    protected function getProcess($commandline, $cwd = null, array $env = null, $stdin = null, $timeout = 60, array $options = array())
+    {
+        $process = new ProcessInSigchildEnvironment($commandline, $cwd, $env, $stdin, $timeout, $options);
+        $process->setEnhanceSigchildCompatibility(true);
+
+        return $process;
+    }
+
+    /**
+     * @expectedException Symfony\Component\Process\Exception\RuntimeException
+     */
+    public function testProcessIsSignaledIfStopped()
+    {
+        parent::testProcessIsSignaledIfStopped();
+    }
+
+    /**
+     * @expectedException Symfony\Component\Process\Exception\RuntimeException
+     */
+    public function testProcessWithTermSignal()
+    {
+        parent::testProcessWithTermSignal();
+    }
+
+    /**
+     * @expectedException Symfony\Component\Process\Exception\RuntimeException
+     */
+    public function testProcessIsNotSignaled()
+    {
+        parent::testProcessIsNotSignaled();
+    }
+
+    /**
+     * @expectedException Symfony\Component\Process\Exception\RuntimeException
+     */
+    public function testProcessWithoutTermSignal()
+    {
+        parent::testProcessWithoutTermSignal();
+    }
+
+    public function testExitCodeText()
+    {
+        $process = $this->getProcess('qdfsmfkqsdfmqmsd');
+        $process->run();
+
+        $this->assertInternalType('string', $process->getExitCodeText());
+    }
+
+}

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

@@ -0,0 +1,87 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Process\Tests;
+
+use Symfony\Component\Process\Process;
+
+class SimpleProcessTest extends AbstractProcessTest
+{
+
+    protected function skipIfPHPSigchild()
+    {
+        ob_start();
+        phpinfo(INFO_GENERAL);
+
+        if (false !== strpos(ob_get_clean(), '--enable-sigchild')) {
+            $this->markTestSkipped('Your PHP has been compiled with --enable-sigchild, this test can not be executed');
+        }
+    }
+
+    protected function getProcess($commandline, $cwd = null, array $env = null, $stdin = null, $timeout = 60, array $options = array())
+    {
+        return new Process($commandline, $cwd, $env, $stdin, $timeout, $options);
+    }
+
+    public function testGetExitCode()
+    {
+        $this->skipIfPHPSigchild();
+        parent::testGetExitCode();
+    }
+
+    public function testExitCodeCommandFailed()
+    {
+        $this->skipIfPHPSigchild();
+        parent::testExitCodeCommandFailed();
+    }
+
+    public function testProcessIsSignaledIfStopped()
+    {
+        $this->skipIfPHPSigchild();
+        parent::testProcessIsSignaledIfStopped();
+    }
+
+    public function testProcessWithTermSignal()
+    {
+        $this->skipIfPHPSigchild();
+        parent::testProcessWithTermSignal();
+    }
+
+    public function testProcessIsNotSignaled()
+    {
+        $this->skipIfPHPSigchild();
+        parent::testProcessIsNotSignaled();
+    }
+
+    public function testProcessWithoutTermSignal()
+    {
+        $this->skipIfPHPSigchild();
+        parent::testProcessWithoutTermSignal();
+    }
+
+    public function testExitCodeText()
+    {
+        $this->skipIfPHPSigchild();
+        parent::testExitCodeText();
+    }
+
+    public function testIsSuccessful()
+    {
+        $this->skipIfPHPSigchild();
+        parent::testIsSuccessful();
+    }
+
+    public function testIsNotSuccessful()
+    {
+        $this->skipIfPHPSigchild();
+        parent::testIsNotSuccessful();
+    }
+}