diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug #10420 [2.3][Process] Make Process::start non-blocking on Windows platform (romainneutron)

Browse Source 
This PR was merged into the 2.3 branch.

Discussion
----------

[2.3][Process] Make Process::start non-blocking on Windows platform

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #9755
| License       | MIT

This PR is a proposition that solves issue #9755.

Let me sum-up what problems we have on Windows platform:
 - We can not use pipes with `proc_open` on Windows because `stream_set_blocking` does not work on this platform, resulting in blocking pipes and a blocking `Process::start` (see https://bugs.php.net/bug.php?id=51800, https://bugs.php.net/bug.php?id=47918 and https://bugs.php.net/bug.php?id=50856).
 - We can not use file handles for both `STDOUT` and `STDERR` as it might produce corrupted content (see https://bugs.php.net/bug.php?id=65650).
 - We currently use file handles for `STDOUT` and pipe for `STDERR` but it makes `Process::start` blocking.

The solution I propose here is to use native redirections, write `STDERR` / `STDOUT` in temporary files, read these files, finally cleanup.
It works pretty well, all tests pass on Windows. Better : [previous tests that were specific to this platform](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Process/Tests/AbstractProcessTest.php#L720-725) disappear as unix one are now okay :).

The drawback of this is the need of using `taskkill`: When stopping a process (`Process::stop` or in case of a timeout reached) `proc_terminate` does not kill the underlying process properly, only the `cmd` parent. The underlying process run by Process still runs after the `proc_terminate` call, having a lock on the temporary files we're using for getting the output, avoiding PHP to remove them (or any user on system) until the underlying process has finish to run).
So I use the `taskkill` Windows command to terminate the underlying process. This works well but I've to admit it's not pretty. If we do not use this hack, let's face that the underlying process is not stopped.

Last but not least, let's deal with the case we're running on Windows platform with PHP having --enable-sigchild environment (I don't know if it can really happen)
In this case:
 - we can not retrieve the `pid`
 - we can not `taskkill` the underlying process
 - it runs
 - locks remain on temporary files, we might not be able to remove them
We can't really deal with this.

Feedback more than welcome

Commits
-------

1f5bf32 [Process] Make Process::start non-blocking on Windows platform
This commit is contained in:
Fabien Potencier 2014-03-18 15:17:47 +01:00
commit 442c4700a5
3 changed files with 63 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/Symfony/Component/Process/Process.php
View File

@ -232,7 +232,11 @@ class Process
$commandline = $this->commandline;
if (defined('PHP_WINDOWS_VERSION_BUILD') && $this->enhanceWindowsCompatibility) {
$commandline = 'cmd /V:ON /E:ON /C "'.$commandline.'"';
$commandline = 'cmd /V:ON /E:ON /C "('.$commandline.')"';
foreach ($this->processPipes->getFiles() as $offset => $filename) {
$commandline .= ' '.$offset.'>'.$filename;
}
if (!isset($this->options['bypass_shell'])) {
$this->options['bypass_shell'] = true;
}
@ -618,6 +622,12 @@ class Process
{
$timeoutMicro = microtime(true) + $timeout;
if ($this->isRunning()) {
if (defined('PHP_WINDOWS_VERSION_BUILD') && !$this->isSigchildEnabled()) {
exec(sprintf("taskkill /F /T /PID %d 2>&1", $this->getPid()), $output, $exitCode);
if ($exitCode > 0) {
throw new RuntimeException('Unable to kill the process');
}
}
proc_terminate($this->process);
do {
usleep(1000);

55
src/Symfony/Component/Process/ProcessPipes.php
View File

@ -21,6 +21,8 @@ class ProcessPipes
/** @var array */
public $pipes = array();
/** @var array */
private $files = array();
/** @var array */
private $fileHandles = array();
/** @var array */
private $readBytes = array();
@ -39,20 +41,21 @@ class ProcessPipes
// Fix for PHP bug #51800: reading from STDOUT pipe hangs forever on Windows if the output is too big.
// Workaround for this problem is to use temporary files instead of pipes on Windows platform.
//
// Please note that this work around prevents hanging but
// another issue occurs : In some race conditions, some data may be
// lost or corrupted.
//
// @see https://bugs.php.net/bug.php?id=51800
if ($this->useFiles) {
$this->fileHandles = array(
Process::STDOUT => tmpfile(),
$this->files = array(
Process::STDOUT => tempnam(sys_get_temp_dir(), 'sf_proc_stdout'),
Process::STDERR => tempnam(sys_get_temp_dir(), 'sf_proc_stderr'),
);
if (false === $this->fileHandles[Process::STDOUT]) {
throw new RuntimeException('A temporary file could not be opened to write the process output to, verify that your TEMP environment variable is writable');
foreach ($this->files as $offset => $file) {
$this->fileHandles[$offset] = fopen($this->files[$offset], 'rb');
if (false === $this->fileHandles[$offset]) {
throw new RuntimeException('A temporary file could not be opened to write the process output to, verify that your TEMP environment variable is writable');
}
}
$this->readBytes = array(
Process::STDOUT => 0,
Process::STDERR => 0,
);
}
}
@ -60,6 +63,7 @@ class ProcessPipes
public function __destruct()
{
$this->close();
$this->removeFiles();
}
/**
@ -105,11 +109,13 @@ class ProcessPipes
public function getDescriptors()
{
if ($this->useFiles) {
// We're not using pipe on Windows platform as it hangs (https://bugs.php.net/bug.php?id=51800)
// We're not using file handles as it can produce corrupted output https://bugs.php.net/bug.php?id=65650
// So we redirect output within the commandline and pass the nul device to the process
return array(
array('pipe', 'r'),
$this->fileHandles[Process::STDOUT],
// Use a file handle only for STDOUT. Using for both STDOUT and STDERR would trigger https://bugs.php.net/bug.php?id=65650
array('pipe', 'w'),
array('file', 'NUL', 'w'),
array('file', 'NUL', 'w'),
);
}
@ -128,6 +134,20 @@ class ProcessPipes
);
}
/**
* Returns an array of filenames indexed by their related stream in case these pipes use temporary files.
*
* @return array
*/
public function getFiles()
{
if ($this->useFiles) {
return $this->files;
}
return array();
}
/**
* Reads data in file handles and pipes.
*
@ -322,4 +342,17 @@ class ProcessPipes
// stream_select returns false when the `select` system call is interrupted by an incoming signal
return isset($lastError['message']) && false !== stripos($lastError['message'], 'interrupted system call');
}
/**
* Removes temporary files
*/
private function removeFiles()
{
foreach ($this->files as $filename) {
if (file_exists($filename)) {
@unlink($filename);
}
}
$this->files = array();
}
}

8
src/Symfony/Component/Process/Tests/SigchildEnabledProcessTest.php
View File

@ -112,6 +112,14 @@ class SigchildEnabledProcessTest extends AbstractProcessTest
$this->markTestSkipped('Signal is not supported in sigchild environment');
}
public function testStartAfterATimeout()
{
if (defined('PHP_WINDOWS_VERSION_BUILD')) {
$this->markTestSkipped('Restarting a timed-out process on Windows is not supported in sigchild environment');
}
parent::testStartAfterATimeout();
}
/**
* {@inheritdoc}
*/