[Process] Don't wait for the process after timeout

nicolas-grekas · nicolas-grekas · commit 7b30620e99ab · 2015-12-07T10:48:10.000+01:00
diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php
@@ -157,16 +157,7 @@ public function __construct($commandline, $cwd = null, array $env = null, $stdin
 
     public function __destruct()
     {
-        if ($this->isRunning()) {
-            $this->doSignal(15, false);
-            usleep(10000);
-        }
-        if ($this->isRunning()) {
-            usleep(100000);
-            $this->doSignal(9, false);
-        }
-
-        // Don't call ->stop() nor ->close() since we don't want to wait for the subprocess here
+        $this->stop(0);
     }
 
     public function __clone()
@@ -678,7 +669,7 @@ public function stop($timeout = 10, $signal = null)
 
         $this->updateStatus(false);
         if ($this->processInformation['running']) {
-            $this->close();
+            $this->close(false);
         }
 
         return $this->exitcode;
@@ -1061,7 +1052,7 @@ protected function updateStatus($blocking)
         $this->readPipes($blocking, '\\' === DIRECTORY_SEPARATOR ? !$this->processInformation['running'] : true);
 
         if (!$this->processInformation['running']) {
-            $this->close();
+            $this->close(true);
         }
     }
 
@@ -1123,12 +1114,14 @@ private function captureExitCode()
     /**
      * Closes process resource, closes file handles, sets the exitcode.
      *
+     * @param bool $wait Whether to wait for the end of the process or not.
+     *
      * @return int The exitcode
      */
-    private function close()
+    private function close($wait)
     {
         $this->processPipes->close();
-        if (is_resource($this->process)) {
+        if ($wait && is_resource($this->process)) {
             $exitcode = proc_close($this->process);
         } else {
             $exitcode = -1;
@@ -1148,6 +1141,7 @@ private function close()
         // Doing so in other contexts like __destruct or by garbage collector is ineffective
         // Now pipes are closed, so the callback is no longer necessary
         $this->callback = null;
+        $this->process = null;
 
         return $this->exitcode;
     }
diff --git a/src/Symfony/Component/Process/Tests/AbstractProcessTest.php b/src/Symfony/Component/Process/Tests/AbstractProcessTest.php
@@ -727,8 +727,7 @@ public function testCheckTimeoutOnStartedProcess()
         }
         $duration = microtime(true) - $start;
 
-        $this->assertLessThan($timeout + $precision, $duration);
-        $this->assertFalse($process->isSuccessful());
+        $this->assertLessThan($timeout + $precision / 1E6, $duration);
 
         throw $e;
     }
diff --git a/src/Symfony/Component/Process/Tests/SigchildDisabledProcessTest.php b/src/Symfony/Component/Process/Tests/SigchildDisabledProcessTest.php
@@ -85,15 +85,6 @@ public function testProcessWithoutTermSignal()
         parent::testProcessWithoutTermSignal();
     }
 
-    /**
-     * @expectedException \Symfony\Component\Process\Exception\RuntimeException
-     * @expectedExceptionMessage This PHP has been compiled with --enable-sigchild. You must use setEnhanceSigchildCompatibility() to use this method.
-     */
-    public function testCheckTimeoutOnStartedProcess()
-    {
-        parent::testCheckTimeoutOnStartedProcess();
-    }
-
     /**
      * @expectedException \Symfony\Component\Process\Exception\RuntimeException
      * @expectedExceptionMessage This PHP has been compiled with --enable-sigchild. The process identifier can not be retrieved.

Original file line number	Diff line number	Diff line change
`@@ -727,8 +727,7 @@ public function testCheckTimeoutOnStartedProcess()`
`727`	`727`	`}`
`728`	`728`	`$duration = microtime(true) - $start;`
`729`	`729`
`730`		`- $this->assertLessThan($timeout + $precision, $duration);`
`731`		`- $this->assertFalse($process->isSuccessful());`
	`730`	`+ $this->assertLessThan($timeout + $precision / 1E6, $duration);`
`732`	`731`
`733`	`732`	`throw $e;`
`734`	`733`	`}`