merged branch romainneutron/fix-8742 (PR #8744)

This PR was merged into the 2.3 branch.

Discussion
----------

[Process] Fix #8742 : Signal-terminated processes are not successful

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | require #8741 to be merged to pass tests
| Fixed tickets | #8742
| License       | MIT

Commits
-------

909fab6 [Process] Fix #8742 : Signal-terminated processes are not successful

src/Symfony/Component/Process/Process.php

@@ -332,8 +332,6 @@ class Process
             usleep(1000);
         }
 
-        $exitcode = proc_close($this->process);
-
         if ($this->processInformation['signaled']) {
             if ($this->isSigchildEnabled()) {
                 throw new RuntimeException('The process has been signaled.');
@@ -342,12 +340,6 @@ class Process
             throw new RuntimeException(sprintf('The process has been signaled with signal "%s".', $this->processInformation['termsig']));
         }
 
-        $this->exitcode = $this->processInformation['running'] ? $exitcode : $this->processInformation['exitcode'];
-
-        if (-1 == $this->exitcode && null !== $this->fallbackExitcode) {
-            $this->exitcode = $this->fallbackExitcode;
-        }
-
         return $this->exitcode;
     }
 
@@ -664,20 +656,7 @@ class Process
                 }
             }
 
-            foreach ($this->pipes as $pipe) {
-                fclose($pipe);
-            }
-            $this->pipes = array();
-
-            $exitcode = proc_close($this->process);
-            $this->exitcode = -1 === $this->processInformation['exitcode'] ? $exitcode : $this->processInformation['exitcode'];
-
-            if (defined('PHP_WINDOWS_VERSION_BUILD')) {
-                foreach ($this->fileHandles as $fileHandle) {
-                    fclose($fileHandle);
-                }
-                $this->fileHandles = array();
-            }
+            $this->updateStatus(false);
         }
         $this->status = self::STATUS_TERMINATED;
 
@@ -1070,11 +1049,10 @@ class Process
         $this->readPipes($blocking);
 
         $this->processInformation = proc_get_status($this->process);
+        $this->captureExitCode();
         if (!$this->processInformation['running']) {
+            $this->close();
             $this->status = self::STATUS_TERMINATED;
-            if (-1 !== $this->processInformation['exitcode']) {
-                $this->exitcode = $this->processInformation['exitcode'];
-            }
         }
     }
 
@@ -1253,4 +1231,53 @@ class Process
             }
         }
     }
+
+    /**
+     * Captures the exitcode if mentioned in the process informations.
+     */
+    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.
+     *
+     * @return Integer The exitcode
+     */
+    private function close()
+    {
+        foreach ($this->pipes as $pipe) {
+            fclose($pipe);
+        }
+
+        $this->pipes = null;
+        $exitcode = -1;
+
+        if (is_resource($this->process)) {
+            $exitcode = proc_close($this->process);
+        }
+
+        $this->exitcode = $this->exitcode !== null ? $this->exitcode : -1;
+        $this->exitcode = -1 != $exitcode ? $exitcode : $this->exitcode;
+
+        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 (defined('PHP_WINDOWS_VERSION_BUILD')) {
+            foreach ($this->fileHandles as $fileHandle) {
+                fclose($fileHandle);
+            }
+            $this->fileHandles = array();
+        }
+
+        return $this->exitcode;
+    }
 }

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

@@ -483,6 +483,24 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals('Caught SIGUSR1', $process->getOutput());
     }
 
+    public function testExitCodeIsAvailableAfterSignal()
+    {
+        $this->verifyPosixIsEnabled();
+
+        $process = $this->getProcess('sleep 4');
+        $process->start();
+        $process->signal(SIGKILL);
+
+        while ($process->isRunning()) {
+            usleep(10000);
+        }
+
+        $this->assertFalse($process->isRunning());
+        $this->assertTrue($process->hasBeenSignaled());
+        $this->assertFalse($process->isSuccessful());
+        $this->assertEquals(137, $process->getExitCode());
+    }
+
     /**
      * @expectedException Symfony\Component\Process\Exception\LogicException
      */

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

@@ -138,6 +138,11 @@ class SigchildDisabledProcessTest extends AbstractProcessTest
         $this->markTestSkipped('Retrieving Pid is not supported in sigchild environment');
     }
 
+    public function testExitCodeIsAvailableAfterSignal()
+    {
+        $this->markTestSkipped('Signal is not supported in sigchild environment');
+    }
+
     /**
      * {@inheritdoc}
      */

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

@@ -98,6 +98,11 @@ class SigchildEnabledProcessTest extends AbstractProcessTest
         $this->markTestSkipped('Retrieving Pid is not supported in sigchild environment');
     }
 
+    public function testExitCodeIsAvailableAfterSignal()
+    {
+        $this->markTestSkipped('Signal is not supported in sigchild environment');
+    }
+
     /**
      * {@inheritdoc}
      */