bug #16915 [Process] Enhance compatiblity with --enable-sigchild (nic…

…olas-grekas) This PR was merged into the 2.3 branch. Discussion ---------- [Process] Enhance compatiblity with --enable-sigchild | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #16888 | License | MIT | Doc PR | - This is complete rewrite of the fallback `--enable-sigchild` handling in the Process class. It removes most of the differences between this and a non-sigchild-enabled php. Which means the test suite doesn't need anymore to be replayed 3 times (which is how I started this PR, looking for a way to test this component in less time). I validated this with a locally compiled php, sigchild-enabled. Green. Changes affect only this special-mode php. Ping @romainneutron and @Seldaek (original writer of the sigchild support) Submitted on 2.3 as bugfix, which it is to me. Commits ------- e7cc4aa [Process] Enhance compatiblity with --enable-sigchild
symfony · Dec 10, 2015 · 423f83f · 423f83f
2 parents 4cde2d1 + e7cc4aa
commit 423f83f
Show file tree

Hide file tree

Showing 8 changed files with 184 additions and 706 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -10,6 +10,7 @@ addons:
 cache:
     directories:
         - .phpunit
+        - php-5.3.9
 
 matrix:
     include:
@@ -32,6 +33,7 @@ env:
 
 before_install:
     - if [[ "$deps" = "no" ]] && [[ "$TRAVIS_PHP_VERSION" =~ 5.[45] ]] && [[ "$TRAVIS_PULL_REQUEST" != "false" ]]; then export deps=skip; fi;
+    - if [[ $deps = no && $TRAVIS_PHP_VERSION = 5.3 && ! -d php-5.3.9/sapi ]]; then wget http://museum.php.net/php5/php-5.3.9.tar.bz2; tar -xjf php-5.3.9.tar.bz2; (cd php-5.3.9; ./configure --enable-sigchild --enable-pcntl; make -j2); fi;
     - if [[ "$TRAVIS_PHP_VERSION" != "hhvm" ]]; then INI_FILE=~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/travis.ini; else INI_FILE=/etc/hhvm/php.ini; fi;
     - echo "memory_limit = -1" >> $INI_FILE
     - echo "session.gc_probability = 0" >> $INI_FILE
@@ -54,6 +56,7 @@ install:
 script:
     - if [ "$deps" = "no" ]; then echo "$COMPONENTS" | parallel --gnu '$PHPUNIT --exclude-group tty,benchmark,intl-data {}'; fi;
     - if [ "$deps" = "no" ]; then echo -e "\\nRunning tests requiring tty"; $PHPUNIT --group tty; fi;
+    - if [[ $deps = no && $TRAVIS_PHP_VERSION = 5.3 ]]; then echo -e "1\\n0" | parallel --gnu 'echo -e "\\nPHP --enable-sigchild enhanced={}" && ENHANCE_SIGCHLD={} php-5.3.9/sapi/cli/php .phpunit/phpunit-4.8/phpunit --colors=always src/Symfony/Component/Process/'; fi;
     - if [ "$deps" = "high" ]; then echo "$COMPONENTS" | parallel --gnu -j10% 'cd {}; composer --prefer-source update; $PHPUNIT --exclude-group tty,benchmark,intl-data'; fi;
     - if [ "$deps" = "low" ]; then echo "$COMPONENTS" | parallel --gnu -j10% 'cd {}; composer --prefer-source --prefer-lowest --prefer-stable update; $PHPUNIT --exclude-group tty,benchmark,intl-data'; fi;
     - if [ "$deps" = "skip" ]; then echo 'This matrix line is skipped for pull requests.'; fi;
diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php
@@ -46,7 +46,7 @@ class Process
     private $timeout;
     private $options;
     private $exitcode;
-    private $fallbackExitcode;
+    private $fallbackStatus = array();
     private $processInformation;
     private $stdout;
     private $stderr;
@@ -65,6 +65,14 @@ class Process
     private $latestSignal;
 
     private static $sigchild;
+    private static $posixSignals = array(
+        1 => 1,   // SIGHUP
+        2 => 2,   // SIGINT
+        3 => 3,   // SIGQUIT
+        6 => 6,   // SIGABRT
+        14 => 14, // SIGALRM
+        15 => 15, // SIGTERM
+    );
 
     /**
      * Exit codes translation table.
@@ -339,17 +347,9 @@ public function wait($callback = null)
      * Returns the Pid (process identifier), if applicable.
      *
      * @return int|null The process id if running, null otherwise
-     *
-     * @throws RuntimeException In case --enable-sigchild is activated
      */
     public function getPid()
     {
-        if ($this->isSigchildEnabled()) {
-            throw new RuntimeException('This PHP has been compiled with --enable-sigchild. The process identifier can not be retrieved.');
-        }
-
-        $this->updateStatus(false);
-
         return $this->isRunning() ? $this->processInformation['pid'] : null;
     }
 
@@ -361,7 +361,7 @@ public function getPid()
      * @return Process
      *
      * @throws LogicException   In case the process is not running
-     * @throws RuntimeException In case --enable-sigchild is activated
+     * @throws RuntimeException In case --enable-sigchild is activated and the process can't be killed
      * @throws RuntimeException In case of failure
      */
     public function signal($signal)
@@ -467,7 +467,9 @@ public function getIncrementalErrorOutput()
      */
     public function getExitCode()
     {
-        if ($this->isSigchildEnabled() && !$this->enhanceSigchildCompatibility) {
+        if (!$this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) {
+            $this->stop(0);
+
             throw new RuntimeException('This PHP has been compiled with --enable-sigchild. You must use setEnhanceSigchildCompatibility() to use this method.');
         }
 
@@ -484,8 +486,6 @@ public function getExitCode()
      *
      * @return null|string A string representation for the exit status code, null if the Process is not terminated.
      *
-     * @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
      */
@@ -522,12 +522,12 @@ public function hasBeenSignaled()
     {
         $this->requireProcessIsTerminated(__FUNCTION__);
 
-        if ($this->isSigchildEnabled()) {
+        if (!$this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) {
+            $this->stop(0);
+
             throw new RuntimeException('This PHP has been compiled with --enable-sigchild. Term signal can not be retrieved.');
         }
 
-        $this->updateStatus(false);
-
         return $this->processInformation['signaled'];
     }
 
@@ -545,12 +545,12 @@ public function getTermSignal()
     {
         $this->requireProcessIsTerminated(__FUNCTION__);
 
-        if ($this->isSigchildEnabled()) {
+        if ($this->isSigchildEnabled() && (!$this->enhanceSigchildCompatibility || -1 === $this->processInformation['termsig'])) {
+            $this->stop(0);
+
             throw new RuntimeException('This PHP has been compiled with --enable-sigchild. Term signal can not be retrieved.');
         }
 
-        $this->updateStatus(false);
-
         return $this->processInformation['termsig'];
     }
 
@@ -567,8 +567,6 @@ public function hasBeenStopped()
     {
         $this->requireProcessIsTerminated(__FUNCTION__);
 
-        $this->updateStatus(false);
-
         return $this->processInformation['stopped'];
     }
 
@@ -585,8 +583,6 @@ public function getStopSignal()
     {
         $this->requireProcessIsTerminated(__FUNCTION__);
 
-        $this->updateStatus(false);
-
         return $this->processInformation['stopsig'];
     }
 
@@ -660,7 +656,7 @@ public function stop($timeout = 10, $signal = null)
                 usleep(1000);
             } while ($this->isRunning() && microtime(true) < $timeoutMicro);
 
-            if ($this->isRunning() && !$this->isSigchildEnabled()) {
+            if ($this->isRunning()) {
                 // Avoid exception here: process is supposed to be running, but it might have stopped just
                 // after this line. In any case, let's silently discard the error, we cannot do anything.
                 $this->doSignal($signal ?: 9, false);
@@ -998,9 +994,15 @@ private function getDescriptors()
 
         if (!$this->useFileHandles && $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')));
+            $descriptors[3] = array('pipe', 'w');
+
+            $trap = '';
+            foreach (self::$posixSignals as $s) {
+                $trap .= "trap 'echo s$s >&3' $s;";
+            }
 
-            $this->commandline = '('.$this->commandline.') 3>/dev/null; code=$?; echo $code >&3; exit $code';
+            $this->commandline = $trap.'{ ('.$this->commandline.') <&3 3<&- 3>/dev/null & } 3<&0;';
+            $this->commandline .= 'pid=$!; echo p$pid >&3; wait $pid; code=$?; echo x$code >&3; exit $code';
         }
 
         return $descriptors;
@@ -1047,10 +1049,13 @@ protected function updateStatus($blocking)
         }
 
         $this->processInformation = proc_get_status($this->process);
-        $this->captureExitCode();
 
         $this->readPipes($blocking, '\\' === DIRECTORY_SEPARATOR ? !$this->processInformation['running'] : true);
 
+        if ($this->fallbackStatus && $this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) {
+            $this->processInformation = $this->fallbackStatus + $this->processInformation;
+        }
+
         if (!$this->processInformation['running']) {
             $this->close();
         }
@@ -1067,7 +1072,7 @@ protected function isSigchildEnabled()
             return self::$sigchild;
         }
 
-        if (!function_exists('phpinfo')) {
+        if (!function_exists('phpinfo') || defined('HHVM_VERSION')) {
             return self::$sigchild = false;
         }
 
@@ -1093,24 +1098,24 @@ private function readPipes($blocking, $close)
 
         $callback = $this->callback;
         foreach ($result as $type => $data) {
-            if (3 == $type) {
-                $this->fallbackExitcode = (int) $data;
+            if (3 === $type) {
+                foreach (explode("\n", substr($data, 0, -1)) as $data) {
+                    if ('p' === $data[0]) {
+                        $this->fallbackStatus['pid'] = (int) substr($data, 1);
+                    } elseif ('s' === $data[0]) {
+                        $this->fallbackStatus['signaled'] = true;
+                        $this->fallbackStatus['exitcode'] = -1;
+                        $this->fallbackStatus['termsig'] = (int) substr($data, 1);
+                    } elseif ('x' === $data[0] && !isset($this->fallbackStatus['signaled'])) {
+                        $this->fallbackStatus['exitcode'] = (int) substr($data, 1);
+                    }
+                }
             } else {
                 $callback($type === self::STDOUT ? self::OUT : self::ERR, $data);
             }
         }
     }
 
-    /**
-     * Captures the exitcode if mentioned in the process information.
-     */
-    private function captureExitCode()
-    {
-        if (isset($this->processInformation['exitcode']) && -1 != $this->processInformation['exitcode']) {
-            $this->exitcode = $this->processInformation['exitcode'];
-        }
-    }
-
     /**
      * Closes process resource, closes file handles, sets the exitcode.
      *
@@ -1120,19 +1125,19 @@ private function close()
     {
         $this->processPipes->close();
         if (is_resource($this->process)) {
-            $exitcode = proc_close($this->process);
-        } else {
-            $exitcode = -1;
+            proc_close($this->process);
         }
-
-        $this->exitcode = -1 !== $exitcode ? $exitcode : (null !== $this->exitcode ? $this->exitcode : -1);
+        $this->exitcode = $this->processInformation['exitcode'];
         $this->status = self::STATUS_TERMINATED;
 
-        if (-1 === $this->exitcode && null !== $this->fallbackExitcode) {
-            $this->exitcode = $this->fallbackExitcode;
-        } elseif (-1 === $this->exitcode && $this->processInformation['signaled'] && 0 < $this->processInformation['termsig']) {
-            // if process has been signaled, no exitcode but a valid termsig, apply Unix convention
-            $this->exitcode = 128 + $this->processInformation['termsig'];
+        if (-1 === $this->exitcode) {
+            if ($this->processInformation['signaled'] && 0 < $this->processInformation['termsig']) {
+                // if process has been signaled, no exitcode but a valid termsig, apply Unix convention
+                $this->exitcode = 128 + $this->processInformation['termsig'];
+            } elseif ($this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) {
+                $this->processInformation['signaled'] = true;
+                $this->processInformation['termsig'] = -1;
+            }
         }
 
         // Free memory from self-reference callback created by buildCallback
@@ -1151,7 +1156,7 @@ private function resetProcessData()
         $this->starttime = null;
         $this->callback = null;
         $this->exitcode = null;
-        $this->fallbackExitcode = null;
+        $this->fallbackStatus = array();
         $this->processInformation = null;
         $this->stdout = null;
         $this->stderr = null;
@@ -1171,7 +1176,7 @@ private function resetProcessData()
      * @return bool 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 --enable-sigchild is activated and the process can't be killed
      * @throws RuntimeException In case of failure
      */
     private function doSignal($signal, $throwException)
@@ -1184,9 +1189,9 @@ private function doSignal($signal, $throwException)
             return false;
         }
 
-        if ($this->isSigchildEnabled()) {
+        if ($this->enhanceSigchildCompatibility && $this->isSigchildEnabled() && !isset(self::$posixSignals[$signal]) && !(function_exists('posix_kill') && @posix_kill($this->getPid(), $signal))) {
             if ($throwException) {
-                throw new RuntimeException('This PHP has been compiled with --enable-sigchild. The process can not be signaled.');
+                throw new RuntimeException('This PHP has been compiled with --enable-sigchild and posix_kill() is not available.');
             }
 
             return false;
@@ -1211,7 +1216,10 @@ private function doSignal($signal, $throwException)
             return false;
         }
 
-        $this->latestSignal = $signal;
+        $this->latestSignal = (int) $signal;
+        $this->fallbackStatus['signaled'] = true;
+        $this->fallbackStatus['exitcode'] = -1;
+        $this->fallbackStatus['termsig'] = $this->latestSignal;
 
         return true;
     }

diff --git a/src/Symfony/Component/Process/Tests/PhpProcessTest.php b/src/Symfony/Component/Process/Tests/PhpProcessTest.php
@@ -30,24 +30,20 @@ public function testNonBlockingWorks()
 
     public function testCommandLine()
     {
-        if ('phpdbg' === PHP_SAPI) {
-            $this->markTestSkipped('phpdbg SAPI is not supported by this test.');
-        }
-
         $process = new PhpProcess(<<<PHP
 <?php echo 'foobar';
 PHP
         );
 
-        $f = new PhpExecutableFinder();
-        $commandLine = $f->find();
+        $commandLine = $process->getCommandLine();
 
-        $this->assertSame($commandLine, $process->getCommandLine(), '::getCommandLine() returns the command line of PHP before start');
+        $f = new PhpExecutableFinder();
+        $this->assertContains($f->find(), $commandLine, '::getCommandLine() returns the command line of PHP before start');
 
         $process->start();
-        $this->assertSame($commandLine, $process->getCommandLine(), '::getCommandLine() returns the command line of PHP after start');
+        $this->assertContains($commandLine, $process->getCommandLine(), '::getCommandLine() returns the command line of PHP after start');
 
         $process->wait();
-        $this->assertSame($commandLine, $process->getCommandLine(), '::getCommandLine() returns the command line of PHP after wait');
+        $this->assertContains($commandLine, $process->getCommandLine(), '::getCommandLine() returns the command line of PHP after wait');
     }
 }
diff --git a/src/Symfony/Component/Process/Tests/ProcessInSigchildEnvironment.php b/src/Symfony/Component/Process/Tests/ProcessInSigchildEnvironment.php