8000 merged branch Seldaek/sigchild-workaround (PR #5353) · symfony/symfony@f11d7a0 · GitHub
[go: up one dir, main page]
Skip to content

Commit f11d7a0

Browse files
fabpotfabpot
committed
merged branch Seldaek/sigchild-workaround (PR #5353)
Commits ------- 7b63428 [Process] Add workaround for PHP's internal sigchild failing to return proper exit codes Discussion ---------- [Process] Add workaround for PHP's internal sigchild failing to return proper exit codes PHP compiled with --enable-sigchild basically fails to return exit codes, and randomly returns -1 instead most of the time (see https://bugs.php.net/bug.php?id=29123). This works around it by having the exit code going through another pipe. It's enabled by default for linux because the new pipe trick won't work on windows I think, but that's unlikely to be an issue because most people don't compile their own php there. I could have it enabled only when sigchild is enabled using the code below, but obviously that adds some overhead, so I'm not sure what's worst. ```php ob_start(); phpinfo(INFO_GENERAL); $sigchild = false !== strpos(ob_get_clean(), '--enable-sigchild'); ``` That said, this renders composer unusable (because we do check exit codes) for people having sigchild enabled, and it's not so easy to workaround outside of the Process class itself, so I hope this is an acceptable fix. --------------------------------------------------------------------------- by schmittjoh at 2012-08-26T13:41:18Z How about prepending commands with ``exec`` to avoid spawning child processes altogether? see #5030 --------------------------------------------------------------------------- by Seldaek at 2012-08-26T13:51:40Z @schmittjoh I'm not sure how that's related to this issue? The problem here is that $exitcode is -1 when it should be 0, I don't see how the additional level of exec would help but maybe I'm missing your point. --------------------------------------------------------------------------- by schmittjoh at 2012-08-26T13:59:15Z I haven't looked in detail at this, but exec removes the child wrapper that PHP adds normally. --------------------------------------------------------------------------- by fabpot at 2012-08-26T16:10:24Z What about doing the fix in 2.0? --------------------------------------------------------------------------- by stof at 2012-08-26T16:13:04Z Can it be applied to 2.0 without too much work ? The Process component has been refactored for 2.1 --------------------------------------------------------------------------- by Seldaek at 2012-08-26T16:16:06Z Just tried to rebase and it's not so trivial.. I can try to rebuild the patch from scratch for 2.0 if it's important. --------------------------------------------------------------------------- by fabpot at 2012-08-26T16:24:38Z ok, let's only do the fix for master for now. --------------------------------------------------------------------------- by Seldaek at 2012-08-26T20:49:39Z @fabpot ok so the question remains whether there should be a ctor check for the configure flag to enable this or if we just always do it and hope it doesn't cause issues.
2 parents 5cc43f9 + 7b63428 commit f11d7a0

File tree

1 file changed

+22
-3
lines changed

1 file changed

+22
-3
lines changed

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

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ class Process
3939
private $timeout;
4040
private $options;
4141
private $exitcode;
42+
private $fallbackExitcode;
4243
private $processInformation;
4344
private $stdout;
4445
private $stderr;
@@ -211,7 +212,13 @@ public function start($callback = null)
211212
);
212213
$descriptors = array(array('pipe', 'r'), $this->fileHandles[self::STDOUT], array('pipe', 'w'));
213214
} else {
214-
$descriptors = array(array('pipe', 'r'), array('pipe', 'w'), array('pipe', 'w'));
215+
$descriptors = array(
216+
array('pipe', 'r'), // stdin
217+
array('pipe', 'w'), // stdout
218+
array('pipe', 'w'), // stderr
219+
array('pipe', 'w') // last exit code is output on the fourth pipe and caught to work around --enable-sigchild
220+
);
221+
$this->commandline = '('.$this->commandline.') 3>/dev/null; echo $? >&3';
215222
}
216223

217224
$commandline = $this->commandline;
@@ -336,8 +343,14 @@ public function wait($callback = null)
336343
foreach ($r as $pipe) {
337344
$type = array_search($pipe, $this->pipes);
338345
$data = fread($pipe, 8192);
346+
339347
if (strlen($data) > 0) {
340-
call_user_func($callback, $type == 1 ? self::OUT : self::ERR, $data);
348+
// last exit code is output and caught to work around --enable-sigchild
349+
if (3 == $type) {
350+
$this->fallbackExitcode = (int) $data;
351+
} else {
352+
call_user_func($callback, $type == 1 ? self::OUT : self::ERR, $data);
353+
}
341354
}
342355
if (false === $data || feof($pipe)) {
343356
fclose($pipe);
@@ -363,7 +376,13 @@ public function wait($callback = null)
363376
throw new \RuntimeException(sprintf('The process stopped because of a "%s" signal.', $this->processInformation['stopsig']));
364377
}
365378

366-
return $this->exitcode = $this->processInformation['running'] ? $exitcode : $this->processInformation['exitcode'];
379+
$this->exitcode = $this->processInformation['running'] ? $exitcode : $this->processInformation['exitcode'];
380+
381+
if (-1 == $this->exitcode && null !== $this->fallbackExitcode) {
382+
$this->exitcode = $this->fallbackExitcode;
383+
}
384+
385+
return $this->exitcode;
367386
}
368387

369388
/**

0 commit comments

Comments
 (0)
0