Merge pull request #63 from clue-labs/sigchild

Improve sigchild compatibility and support explicit configuration

Author: WyriHaximus

README.md

@@ -295,17 +295,39 @@ timeout.
 
 ### Sigchild Compatibility
 
-When PHP has been compiled with the `--enabled-sigchild` option, a child
-process' exit code cannot be reliably determined via `proc_close()` or
-`proc_get_status()`. Instead, we execute the child process with a fourth pipe
-and use that to retrieve its exit code.
+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:
 
-This behavior is used by default and only when necessary. It may be manually
-disabled by calling `setEnhanceSigchildCompatibility(false)` on the Process
-before it is started, in which case the `exit` event may receive `null` instead
-of the actual exit code.
+```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:** This functionality was taken from Symfony's
+**Note:** The original functionality was taken from Symfony's
 [Process](https://github.com/symfony/process) compoment.
 
 ### Windows Compatibility

src/Process.php

@@ -64,7 +64,7 @@ public function __construct($cmd, $cwd = null, array $env = null)
             }
         }
 
-        $this->enhanceSigchildCompatibility = $this->isSigchildEnabled();
+        $this->enhanceSigchildCompatibility = self::isSigchildEnabled();
     }
 
     /**
@@ -91,7 +91,7 @@ public function start(LoopInterface $loop, $interval = 0.1)
         );
 
         // Read exit code through fourth pipe to work around --enable-sigchild
-        if ($this->isSigchildEnabled() && $this->enhanceSigchildCompatibility) {
+        if ($this->enhanceSigchildCompatibility) {
             $fdSpec[] = array('pipe', 'w');
             $cmd = sprintf('(%s) 3>/dev/null; code=$?; echo $code >&3; exit $code', $cmd);
         }
@@ -152,7 +152,7 @@ public function close()
         $this->stdout->close();
         $this->stderr->close();
 
-        if ($this->isSigchildEnabled() && $this->enhanceSigchildCompatibility) {
+        if ($this->enhanceSigchildCompatibility) {
             $this->pollExitCodePipe();
             $this->closeExitCodePipe();
         }
@@ -203,38 +203,6 @@ public function getCommand()
         return $this->cmd;
     }
 
-    /**
-     * Return whether sigchild compatibility is enabled.
-     *
-     * @return boolean
-     */
-    public final function getEnhanceSigchildCompatibility()
-    {
-        return $this->enhanceSigchildCompatibility;
-    }
-
-    /**
-     * 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 boolean $enhance
-     * @return self
-     * @throws RuntimeException If the process is already running
-     */
-    public final function setEnhanceSigchildCompatibility($enhance)
-    {
-        if ($this->isRunning()) {
-            throw new \RuntimeException('Process is already running');
-        }
-
-        $this->enhanceSigchildCompatibility = (bool) $enhance;
-
-        return $this;
-    }
-
     /**
      * Get the exit code returned by the process.
      *
@@ -347,6 +315,21 @@ public final static function isSigchildEnabled()
         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 boolean $sigchild
+     * @return void
+     */
+    public final static function setSigchildEnabled($sigchild)
+    {
+        self::$sigchild = (bool) $sigchild;
+    }
+
     /**
      * Check the fourth pipe for an exit code.
      *

tests/AbstractProcessTest.php

@@ -11,28 +11,6 @@ abstract class AbstractProcessTest extends TestCase
 {
     abstract public function createLoop();
 
-    public function testGetEnhanceSigchildCompatibility()
-    {
-        $process = new Process('echo foo');
-
-        $this->assertSame($process, $process->setEnhanceSigchildCompatibility(true));
-        $this->assertTrue($process->getEnhanceSigchildCompatibility());
-
-        $this->assertSame($process, $process->setEnhanceSigchildCompatibility(false));
-        $this->assertFalse($process->getEnhanceSigchildCompatibility());
-    }
-
-    /**
-     * @expectedException RuntimeException
-     */
-    public function testSetEnhanceSigchildCompatibilityCannotBeCalledIfProcessIsRunning()
-    {
-        $process = new Process('sleep 1');
-
-        $process->start($this->createLoop());
-        $process->setEnhanceSigchildCompatibility(false);
-    }
-
     public function testGetCommand()
     {
         $process = new Process('echo foo');
@@ -67,6 +45,20 @@ public function testGetTermSignalWhenRunning($process)
         $this->assertNull($process->getTermSignal());
     }
 
+    public function testCommandWithEnhancedSigchildCompatibilityReceivesExitCode()
+    {
+        $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()
     {
         $cmd = 'echo test';