8000 bug #17276 [Process] Fix potential race condition (nicolas-grekas) · symfony/symfony@707d359 · GitHub
[go: up one dir, main page]

Skip to content
8000

Commit 707d359

Browse files
bug #17276 [Process] Fix potential race condition (nicolas-grekas)
This PR was mer 8000 ged into the 2.3 branch. Discussion ---------- [Process] Fix potential race condition | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - As seen at https://travis-ci.org/symfony/symfony/jobs/100448880#L2768, ProcessTest::testRunProcessWithTimeout can create an infinite blocking situation. I found two potential code paths that could be subject to a race condition. This fixes them. Commits ------- b114a85 [Process] Fix potential race condition
2 parents e9b8aae + b114a85 commit 707d359

File tree

2 files changed

+13
-15
lines changed

2 files changed

+13
-15
lines changed

src/Symfony/Component/Process/Process.php

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -651,8 +651,12 @@ public function stop($timeout = 10, $signal = null)
651651
}
652652
}
653653

654-
$this->updateStatus(false);
655-
if ($this->processInformation['running']) {
654+
if ($this->isRunning()) {
655+
if (isset($this->fallbackStatus['pid'])) {
656+
unset($this->fallbackStatus['pid']);
657+
658+
return $this->stop(0, $signal);
659+
}
656660
$this->close();
657661
}
658662

@@ -1145,7 +1149,7 @@ private function resetProcessData()
11451149
*/
11461150
private function doSignal($signal, $throwException)
11471151
{
1148-
if (!$this->isRunning()) {
1152+
if (null === $pid = $this->getPid()) {
11491153
if ($throwException) {
11501154
throw new LogicException('Can not send signal on a non running process.');
11511155
}
@@ -1154,7 +1158,7 @@ private function doSignal($signal, $throwException)
11541158
}
11551159

11561160
if ('\\' === DIRECTORY_SEPARATOR) {
1157-
exec(sprintf('taskkill /F /T /PID %d 2>&1', $this->getPid()), $output, $exitCode);
1161+
exec(sprintf('taskkill /F /T /PID %d 2>&1', $pid), $output, $exitCode);
11581162
if ($exitCode && $this->isRunning()) {
11591163
if ($throwException) {
11601164
throw new RuntimeException(sprintf('Unable to kill the process (%s).', implode(' ', $output)));
@@ -1166,8 +1170,8 @@ private function doSignal($signal, $throwException)
11661170
if (!$this->enhanceSigchildCompatibility || !$this->isSigchildEnabled()) {
11671171
$ok = @proc_terminate($this->process, $signal);
11681172
} elseif (function_exists('posix_kill')) {
1169-
$ok = @posix_kill($this->getPid(), $signal);
1170-
} elseif ($ok = proc_open(sprintf('kill -%d %d', $signal, $this->getPid()), array(2 => array('pipe', 'w')), $pipes)) {
1173+
$ok = @posix_kill($pid, $signal);
1174+
} elseif ($ok = proc_open(sprintf('kill -%d %d', $signal, $pid), array(2 => array('pipe', 'w')), $pipes)) {
11711175
$ok = false === fgets($pipes[2]);
11721176
}
11731177
if (!$ok) {

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

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -685,22 +685,16 @@ public function testRestart()
685685
*/
686686
public function testRunProcessWithTimeout()
687687
{
688-
$timeout = 0.1;
689-
$process = $this->getProcess(self::$phpBin.' -r "sleep(1);"');
690-
$process->setTimeout($timeout);
688+
$process = $this->getProcess(self::$phpBin.' -r "sleep(30);"');
689+
$process->setTimeout(0.1);
691690
$start = microtime(true);
692691
try {
693692
$process->run();
694693
$this->fail('A RuntimeException should have been raised');
695694
} catch (RuntimeException $e) {
696695
}
697-
$duration = microtime(true) - $start;
698696

699-
if ('\\' !== DIRECTORY_SEPARATOR) {
700-
// On Windows, timers are too transient
701-
$maxDuration = $timeout + 2 * Process::TIMEOUT_PRECISION;
702-
$this->assertLessThan($maxDuration, $duration);
703-
}
697+
$this->assertLessThan(15, microtime(true) - $start);
704698

705699
throw $e;
706700
}

0 commit comments

Comments
 (0)
0