Skip to content

Commit 80fb51c

Browse files
nicolas-grekasnicolas-grekas
committed
[Process] Fix stopping a process on Windows
1 parent d9b8d0c commit 80fb51c

File tree

4 files changed

+54
-24
lines changed

4 files changed

+54
-24
lines changed

‎src/Symfony/Component/Process/Process.php

+11-3
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,16 @@ public function __construct($commandline, $cwd = null, array $env = null, $stdin
157157

158158
public function __destruct()
159159
{
160-
// stop() will check if we have a process running.
161-
$this->stop();
160+
if ($this->isRunning()) {
161+
$this->doSignal(15, false);
162+
usleep(10000);
163+
}
164+
if ($this->isRunning()) {
165+
usleep(100000);
166+
$this->doSignal(9, false);
167+
}
168+
169+
// Don't call ->stop() nor ->close() since we don't want to wait for the subprocess here
162170
}
163171

164172
public function __clone()
@@ -1190,7 +1198,7 @@ private function doSignal($signal, $throwException)
11901198

11911199
if ('\\' === DIRECTORY_SEPARATOR) {
11921200
exec(sprintf('taskkill /F /T /PID %d 2>&1', $this->getPid()), $output, $exitCode);
1193-
if ($exitCode) {
1201+
if ($exitCode && $this->isRunning()) {
11941202
if ($throwException) {
11951203
throw new RuntimeException(sprintf('Unable to kill the process (%s).', implode(' ', $output)));
11961204
}

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

+35-13
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,10 @@ public function testProcessPipes($code, $size)
167167
$this->assertEquals($expectedLength, strlen($p->getErrorOutput()));
168168
}
169169

170+
/**
171+
* @expectedException Symfony\Component\Process\Exception\LogicException
172+
* @expectedExceptionMessage STDIN can not be set while the process is running.
173+
*/
170174
public function testSetStdinWhileRunningThrowsAnException()
171175
{
172176
$process = $this->getProcess(self::$phpBin.' -r "usleep(500000);"');
@@ -176,9 +180,10 @@ public function testSetStdinWhileRunningThrowsAnException()
176180
$process->stop();
177181
$this->fail('A LogicException should have been raised.');
178182
} catch (LogicException $e) {
179-
$this->assertEquals('STDIN can not be set while the process is running.', $e->getMessage());
180183
}
181184
$process->stop();
185+
186+
throw $e;
182187
}
183188

184189
/**
@@ -659,6 +664,10 @@ public function testRestart()
659664
$this->assertNotEquals($process1->getOutput(), $process2->getOutput());
660665
}
661666

667+
/**
668+
* @expectedException Symfony\Component\Process\Exception\RuntimeException
669+
* @expectedExceptionMessage The process timed-out.
670+
*/
662671
public function testRunProcessWithTimeout()
663672
{
664673
$timeout = 0.5;
@@ -672,14 +681,13 @@ public function testRunProcessWithTimeout()
672681
}
673682
$duration = microtime(true) - $start;
674683

675-
if ('\\' === DIRECTORY_SEPARATOR) {
676-
// Windows is a bit slower as it read file handles, then allow twice the precision
677-
$maxDuration = $timeout + 2 * Process::TIMEOUT_PRECISION;
678-
} else {
684+
if ('\\' !== DIRECTORY_SEPARATOR) {
685+
// On Windows, timers are too transient
679686
$maxDuration = $timeout + Process::TIMEOUT_PRECISION;
687+
$this->assertLessThan($maxDuration, $duration);
680688
}
681689

682-
$this->assertLessThan($maxDuration, $duration);
690+
throw $e;
683691
}
684692

685693
public function testCheckTimeoutOnNonStartedProcess()
@@ -695,6 +703,10 @@ public function testCheckTimeoutOnTerminatedProcess()
695703
$process->checkTimeout();
696704
}
697705

706+
/**
707+
* @expectedException Symfony\Component\Process\Exception\RuntimeException
708+
* @expectedExceptionMessage The process timed-out.
709+
*/
698710
public function testCheckTimeoutOnStartedProcess()
699711
{
700712
$timeout = 0.5;
@@ -717,8 +729,14 @@ public function testCheckTimeoutOnStartedProcess()
717729

718730
$this->assertLessThan($timeout + $precision, $duration);
719731
$this->assertFalse($process->isSuccessful());
732+
733+
throw $e;
720734
}
721735

736+
/**
737+
* @expectedException Symfony\Component\Process\Exception\RuntimeException
738+
* @expectedExceptionMessage The process timed-out.
739+
*/
722740
public function testStartAfterATimeout()
723741
{
724742
$process = $this->getProcess(sprintf('%s -r %s', self::$phpBin, escapeshellarg('$n = 1000; while ($n--) {echo \'\'; usleep(1000); }')));
@@ -731,6 +749,8 @@ public function testStartAfterATimeout()
731749
$process->start();
732750
usleep(10000);
733751
$process->stop();
752+
753+
throw $e;
734754
}
735755

736756
public function testGetPid()
@@ -760,14 +780,14 @@ public function testSignal()
760780
$this->markTestSkipped('Extension pcntl is required.');
761781
}
762782

763-
$process = $this->getProcess('exec php -f '.__DIR__.'/SignalListener.php');
783+
$process = $this->getProcess('exec '.self::$phpBin.' '.__DIR__.'/SignalListener.php');
764784
$process->start();
765-
usleep(500000);
766-
$process->signal(SIGUSR1);
767785

768-
while ($process->isRunning() && false === strpos($process->getOutput(), 'Caught SIGUSR1')) {
769-
usleep(10000);
786+
while (false === strpos($process->getOutput(), 'Caught')) {
787+
usleep(1000);
770788
}
789+
$process->signal(SIGUSR1);
790+
$process->wait();
771791

772792
$this->assertEquals('Caught SIGUSR1', $process->getOutput());
773793
}
@@ -828,6 +848,8 @@ public function provideMethodsThatNeedARunningProcess()
828848

829849
/**
830850
* @dataProvider provideMethodsThatNeedATerminatedProcess
851+
* @expectedException Symfony\Component\Process\Exception\LogicException
852+
* @expectedExceptionMessage Process must be terminated before calling
831853
*/
832854
public function testMethodsThatNeedATerminatedProcess($method)
833855
{
@@ -838,10 +860,10 @@ public function testMethodsThatNeedATerminatedProcess($method)
838860
$process->stop(0);
839861
$this->fail('A LogicException must have been thrown');
840862
} catch (\Exception $e) {
841-
$this->assertInstanceOf('Symfony\Component\Process\Exception\LogicException', $e);
842-
$this->assertEquals(sprintf('Process must be terminated before calling %s.', $method), $e->getMessage());
843863
}
844864
$process->stop(0);
865+
866+
throw $e;
845867
}
846868

847869
public function provideMethodsThatNeedATerminatedProcess()

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

+4
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,10 @@ public function testExitCodeIsAvailableAfterSignal()
112112
$this->markTestSkipped('Signal is not supported in sigchild environment');
113113
}
114114

115+
/**
116+
* @expectedException Symfony\Component\Process\Exception\RuntimeException
117+
* @expectedExceptionMessage The process timed-out.
118+
*/
115119
public function testStartAfterATimeout()
116120
{
117121
if ('\\' === DIRECTORY_SEPARATOR) {

‎src/Symfony/Component/Process/Tests/SignalListener.php

+4-8
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,13 @@
99
* file that was distributed with this source code.
1010
*/
1111

12-
// required for signal handling
13-
declare (ticks = 1);
12+
pcntl_signal(SIGUSR1, function () {echo 'SIGUSR1'; exit;});
1413

15-
pcntl_signal(SIGUSR1, function () {echo 'Caught SIGUSR1'; exit;});
14+
echo 'Caught ';
1615

1716
$n = 0;
1817

19-
// ticks require activity to work - sleep(4); does not work
20-
while ($n < 400) {
18+
while ($n++ < 400) {
2119
usleep(10000);
22-
++$n;
20+
pcntl_signal_dispatch();
2321
}
24-
25-
return;

0 commit comments

Comments
 (0)