Merge pull request #103 from clue-labs/no-sigchild

Remove legacy sigchild compatibility layer

Author: SimonFrings

README.md

@@ -30,7 +30,6 @@ as [Streams](https://github.com/reactphp/stream).
   * [Command](#command)
   * [Termination](#termination)
   * [Custom pipes](#custom-pipes)
-  * [Sigchild Compatibility](#sigchild-compatibility)
   * [Windows Compatibility](#windows-compatibility)
 * [Install](#install)
 * [Tests](#tests)
@@ -262,8 +261,7 @@ $process->on('exit', function ($code, $term) {
 ```
 
 Note that `$code` is `null` if the process has terminated, but the exit
-code could not be determined (for example
-[sigchild compatibility](#sigchild-compatibility) was disabled).
+code could not be determined.
 Similarly, `$term` is `null` unless the process has terminated in response to
 an uncaught signal sent to it.
 This is not a limitation of this project, but actual how exit codes and signals
@@ -387,43 +385,6 @@ number of pipes and additional file descriptors, but many common applications
 being run as a child process will expect that the parent process properly
 assigns these file descriptors.
 
-### Sigchild Compatibility
-
-Internally, this project uses a work-around to improve compatibility when PHP
-has been compiled with the `--enable-sigchild` option. This should not affect most
-installations as this configure option is not used by default and many
-distributions (such as Debian and Ubuntu) are known to not use this by default.
-Some installations that use [Oracle OCI8](http://php.net/manual/en/book.oci8.php)
-may use this configure option to circumvent `defunct` processes.
-
-When PHP has been compiled with the `--enable-sigchild` option, a child process'
-exit code cannot be reliably determined via `proc_close()` or `proc_get_status()`.
-To work around this, we execute the child process with an additional pipe and
-use that to retrieve its exit code.
-
-This work-around incurs some overhead, so we only trigger this when necessary
-and when we detect that PHP has been compiled with the `--enable-sigchild` option.
-Because PHP does not provide a way to reliably detect this option, we try to
-inspect output of PHP's configure options from the `phpinfo()` function.
-
-The static `setSigchildEnabled(bool $sigchild): void` method can be used to
-explicitly enable or disable this behavior like this:
-
-```php
-// advanced: not recommended by default
-Process::setSigchildEnabled(true);
-```
-
-Note that all processes instantiated after this method call will be affected.
-If this work-around is disabled on an affected PHP installation, the `exit`
-event may receive `null` instead of the actual exit code as described above.
-Similarly, some distributions are known to omit the configure options from
-`phpinfo()`, so automatic detection may fail to enable this work-around in some
-cases. You may then enable this  explicitly as given above.
-
-**Note:** The original functionality was taken from Symfony's
-[Process](https://github.com/symfony/process) compoment.
-
 ### Windows Compatibility
 
 Due to platform constraints, this library provides only limited support for
@@ -607,6 +568,14 @@ This project aims to run on any platform and thus does not require any PHP
 extensions and supports running on legacy PHP 5.3 through current PHP 8+ and HHVM.
 It's *highly recommended to use the latest supported PHP version* for this project.
 
+Note that legacy platforms that use PHP compiled with the legacy `--enable-sigchild`
+option may not reliably determine the child process' exit code in some cases.
+This should not affect most installations as this configure option is not used
+by default and most distributions (such as Debian and Ubuntu) are known to not
+use this by default. This option may be used on some  installations that use
+[Oracle OCI8](https://www.php.net/manual/en/book.oci8.php), see `phpinfo()` output
+to check if your installation might be affected.
+
 See above note for limited [Windows Compatibility](#windows-compatibility).
 
 ## Tests

src/Process.php

@@ -15,9 +15,6 @@
 /**
  * Process component.
  *
- * This class borrows logic from Symfony's Process component for ensuring
- * compatibility when PHP is compiled with the --enable-sigchild option.
- *
  * This class also implements the `EventEmitterInterface`
  * which allows you to react to certain events:
  *
@@ -40,8 +37,7 @@
  *     ```
  *
  *     Note that `$code` is `null` if the process has terminated, but the exit
- *     code could not be determined (for example
- *     [sigchild compatibility](#sigchild-compatibility) was disabled).
+ *     code could not be determined.
  *     Similarly, `$term` is `null` unless the process has terminated in response to
  *     an uncaught signal sent to it.
  *     This is not a limitation of this project, but actual how exit codes and signals
@@ -91,18 +87,13 @@ class Process extends EventEmitter
     private $env;
     private $fds;
 
-    private $enhanceSigchildCompatibility;
-    private $sigchildPipe;
-
     private $process;
     private $status;
     private $exitCode;
     private $fallbackExitCode;
     private $stopSignal;
     private $termSignal;
 
-    private static $sigchild;
-
     /**
     * Constructor.
     *
@@ -145,7 +136,6 @@ public function __construct($cmd, $cwd = null, array $env = null, array $fds = n
         }
 
         $this->fds = $fds;
-        $this->enhanceSigchildCompatibility = self::isSigchildEnabled();
     }
 
     /**
@@ -173,23 +163,6 @@ public function start(LoopInterface $loop = null, $interval = 0.1)
         $loop = $loop ?: Loop::get();
         $cmd = $this->cmd;
         $fdSpec = $this->fds;
-        $sigchild = null;
-
-        // Read exit code through fourth pipe to work around --enable-sigchild
-        if ($this->enhanceSigchildCompatibility) {
-            $fdSpec[] = array('pipe', 'w');
-            \end($fdSpec);
-            $sigchild = \key($fdSpec);
-
-            // make sure this is fourth or higher (do not mess with STDIO)
-            if ($sigchild < 3) {
-                $fdSpec[3] = $fdSpec[$sigchild];
-                unset($fdSpec[$sigchild]);
-                $sigchild = 3;
-            }
-
-            $cmd = \sprintf('(%s) ' . $sigchild . '>/dev/null; code=$?; echo $code >&' . $sigchild . '; exit $code', $cmd);
-        }
 
         // on Windows, we do not launch the given command line in a shell (cmd.exe) by default and omit any error dialogs
         // the cmd.exe shell can explicitly be given as part of the command as detailed in both documentation and tests
@@ -242,11 +215,6 @@ public function start(LoopInterface $loop = null, $interval = 0.1)
             });
         };
 
-        if ($sigchild !== null) {
-            $this->sigchildPipe = $pipes[$sigchild];
-            unset($pipes[$sigchild]);
-        }
-
         foreach ($pipes as $n => $fd) {
             // use open mode from stream meta data or fall back to pipe open mode for legacy HHVM
             $meta = \stream_get_meta_data($fd);
@@ -292,11 +260,6 @@ public function close()
             $pipe->close();
         }
 
-        if ($this->enhanceSigchildCompatibility) {
-            $this->pollExitCodePipe();
-            $this->closeExitCodePipe();
-        }
-
         $exitCode = \proc_close($this->process);
         $this->process = null;
 
@@ -350,7 +313,7 @@ public function getCommand()
      * will be returned if the process is still running.
      *
      * Null may also be returned if the process has terminated, but the exit
-     * code could not be determined (e.g. sigchild compatibility was disabled).
+     * code could not be determined.
      *
      * @return int|null
      */
@@ -437,85 +400,6 @@ public function isTerminated()
         return $status !== null ? $status['signaled'] : false;
     }
 
-    /**
-     * Return whether PHP has been compiled with the '--enable-sigchild' option.
-     *
-     * @see \Symfony\Component\Process\Process::isSigchildEnabled()
-     * @return bool
-     */
-    public final static function isSigchildEnabled()
-    {
-        if (null !== self::$sigchild) {
-            return self::$sigchild;
-        }
-
-        if (!\function_exists('phpinfo')) {
-            return self::$sigchild = false; // @codeCoverageIgnore
-        }
-
-        \ob_start();
-        \phpinfo(INFO_GENERAL);
-
-        return self::$sigchild = false !== \strpos(\ob_get_clean(), '--enable-sigchild');
-    }
-
-    /**
-     * Enable or disable 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 bool $sigchild
-     * @return void
-     */
-    public final static function setSigchildEnabled($sigchild)
-    {
-        self::$sigchild = (bool) $sigchild;
-    }
-
-    /**
-     * Check the fourth pipe for an exit code.
-     *
-     * This should only be used if --enable-sigchild compatibility was enabled.
-     */
-    private function pollExitCodePipe()
-    {
-        if ($this->sigchildPipe === null) {
-            return;
-        }
-
-        $r = array($this->sigchildPipe);
-        $w = $e = null;
-
-        $n = @\stream_select($r, $w, $e, 0);
-
-        if (1 !== $n) {
-            return;
-        }
-
-        $data = \fread($r[0], 8192);
-
-        if (\strlen($data) > 0) {
-            $this->fallbackExitCode = (int) $data;
-        }
-    }
-
-    /**
-     * Close the fourth pipe used to relay an exit code.
-     *
-     * This should only be used if --enable-sigchild compatibility was enabled.
-     */
-    private function closeExitCodePipe()
-    {
-        if ($this->sigchildPipe === null) {
-            return;
-        }
-
-        \fclose($this->sigchildPipe);
-        $this->sigchildPipe = null;
-    }
-
     /**
      * Return the cached process status.
      *

tests/AbstractProcessTest.php

@@ -246,24 +246,6 @@ public function testGetTermSignalWhenRunning($process)
         $this->assertNull($process->getTermSignal());
     }
 
-    public function testCommandWithEnhancedSigchildCompatibilityReceivesExitCode()
-    {
-        if (DIRECTORY_SEPARATOR === '\\') {
-            $this->markTestSkipped('Process pipes not supported on Windows');
-        }
-
-        $loop = $this->createLoop();
-        $old = Process::isSigchildEnabled();
-        Process::setSigchildEnabled(true);
-        $process = new Process('echo foo');
-        $process->start($loop);
-        Process::setSigchildEnabled($old);
-
-        $loop->run();
-
-        $this->assertEquals(0, $process->getExitCode());
-    }
-
     public function testReceivesProcessStdoutFromEcho()
     {
         if (DIRECTORY_SEPARATOR === '\\') {