merged branch Seldaek/process_fix (PR #3838)

Commits ------- 6dca141 [Process] Skip signal assertion on windows 4cd0fb4 [Process] Skip test that is still getting stuck on windows e82a05d [Process] Close pipes before calling proc_close to avoid deadlocks as advised on the proc_close php.net documentation f4227b5 [Process] Removing useless code (this is already done in updateStatus) 2d586d2 [Process] Fix a mistake triggering stream_select errors Discussion ---------- Process fixes A few fixes, including making the tests pass on windows and fixing composer/composer#543. Given the sensitivity of this code I did a bunch of very granular commits explaining everything.
symfony · Apr 8, 2012 · 127cff0 · 127cff0
2 parents d694858 + 6dca141
commit 127cff0
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 8 deletions.
diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php
@@ -213,14 +213,15 @@ public function start($callback = null)
 
         if (null === $this->stdin) {
             fclose($this->pipes[0]);
+            unset($this->pipes[0]);
 
             return;
         } else {
             $writePipes = array($this->pipes[0]);
+            unset($this->pipes[0]);
             $stdinLen = strlen($this->stdin);
             $stdinOffset = 0;
         }
-        unset($this->pipes[0]);
 
         while ($writePipes) {
             $r = $this->pipes;
@@ -480,10 +481,6 @@ public function isRunning()
 
         $this->updateStatus();
 
-        if ($this->processInformation['running'] === false) {
-            $this->status = self::STATUS_TERMINATED;
-        }
-
         return $this->processInformation['running'];
     }
 
@@ -506,6 +503,12 @@ public function stop($timeout=10)
                 $time += 1000;
                 usleep(1000);
             }
+
+            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'];
         }

diff --git a/src/Symfony/Component/Process/Tests/ProcessTest.php b/src/Symfony/Component/Process/Tests/ProcessTest.php
@@ -55,8 +55,8 @@ public function testProcessResponses($expected, $getter, $code)
      */
     public function testProcessPipes($expected, $code)
     {
-        if (strpos(PHP_OS, "WIN") === 0 && version_compare(phpversion(), "5.3.9", "<")) {
-            $this->markTestSkipped('Test hangs on Windows & PHP due to https://bugs.php.net/bug.php?id=60120 fixed in http://svn.php.net/viewvc?view=revision&revision=318366');
+        if (strpos(PHP_OS, "WIN") === 0) {
+            $this->markTestSkipped('Test hangs on Windows & PHP due to https://bugs.php.net/bug.php?id=60120 and https://bugs.php.net/bug.php?id=51800');
         }
 
         $p = new Process(sprintf('php -r %s', escapeshellarg($code)));
@@ -114,7 +114,11 @@ public function testStop()
         $this->assertTrue($process->isRunning());
         $process->stop();
         $this->assertFalse($process->isRunning());
-        $this->assertTrue($process->hasBeenSignaled());
+
+        // skip this check on windows since it does not support signals
+        if (!defined('PHP_WINDOWS_VERSION_MAJOR')) {
+            $this->assertTrue($process->hasBeenSignaled());
+        }
     }
 
     public function responsesCodeProvider()