[Process] Fix potential race condition

symfony · Jan 6, 2016 · dc9db75 · dc9db75
1 parent e9b8aae
commit dc9db75
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 15 deletions.
diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php
@@ -651,8 +651,12 @@ public function stop($timeout = 10, $signal = null)
             }
         }
 
-        $this->updateStatus(false);
-        if ($this->processInformation['running']) {
+        if ($this->isRunning()) {
+            if (isset($this->fallbackStatus['pid'])) {
+                unset($this->fallbackStatus['pid']);
+
+                return $this->stop(0, $signal);
+            }
             $this->close();
         }
 
@@ -1145,7 +1149,7 @@ private function resetProcessData()
      */
     private function doSignal($signal, $throwException)
     {
-        if (!$this->isRunning()) {
+        if (null === $pid = $this->getPid()) {
             if ($throwException) {
                 throw new LogicException('Can not send signal on a non running process.');
             }
@@ -1154,7 +1158,7 @@ private function doSignal($signal, $throwException)
         }
 
         if ('\\' === DIRECTORY_SEPARATOR) {
-            exec(sprintf('taskkill /F /T /PID %d 2>&1', $this->getPid()), $output, $exitCode);
+            exec(sprintf('taskkill /F /T /PID %d 2>&1', $pid), $output, $exitCode);
             if ($exitCode && $this->isRunning()) {
                 if ($throwException) {
                     throw new RuntimeException(sprintf('Unable to kill the process (%s).', implode(' ', $output)));
@@ -1166,8 +1170,8 @@ private function doSignal($signal, $throwException)
             if (!$this->enhanceSigchildCompatibility || !$this->isSigchildEnabled()) {
                 $ok = @proc_terminate($this->process, $signal);
             } elseif (function_exists('posix_kill')) {
-                $ok = @posix_kill($this->getPid(), $signal);
-            } elseif ($ok = proc_open(sprintf('kill -%d %d', $signal, $this->getPid()), array(2 => array('pipe', 'w')), $pipes)) {
+                $ok = @posix_kill($pid, $signal);
+            } elseif ($ok = proc_open(sprintf('kill -%d %d', $signal, $pid), array(2 => array('pipe', 'w')), $pipes)) {
                 $ok = false === fgets($pipes[2]);
             }
             if (!$ok) {

diff --git a/src/Symfony/Component/Process/Tests/ProcessTest.php b/src/Symfony/Component/Process/Tests/ProcessTest.php
@@ -685,22 +685,16 @@ public function testRestart()
      */
     public function testRunProcessWithTimeout()
     {
-        $timeout = 0.1;
-        $process = $this->getProcess(self::$phpBin.' -r "sleep(1);"');
-        $process->setTimeout($timeout);
+        $process = $this->getProcess(self::$phpBin.' -r "sleep(30);"');
+        $process->setTimeout(0.1);
         $start = microtime(true);
         try {
             $process->run();
             $this->fail('A RuntimeException should have been raised');
         } catch (RuntimeException $e) {
         }
-        $duration = microtime(true) - $start;
 
-        if ('\\' !== DIRECTORY_SEPARATOR) {
-            // On Windows, timers are too transient
-            $maxDuration = $timeout + 2 * Process::TIMEOUT_PRECISION;
-            $this->assertLessThan($maxDuration, $duration);
-        }
+        $this->assertLessThan(15, microtime(true) - $start);
 
         throw $e;
     }