Skip to content

[Process] Add feature "wait until callback" to process class #27742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Symfony/Component/Process/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ CHANGELOG
* added the `Process::fromShellCommandline()` to run commands in a shell wrapper
* deprecated passing a command as string when creating a `Process` instance
* deprecated the `Process::setCommandline()` and the `PhpProcess::setPhpBinary()` methods
* added the `Process::waitUntil()` method to wait for the process only for a
specific output, then continue the normal execution of your application

4.1.0
-----
Expand Down
45 changes: 42 additions & 3 deletions src/Symfony/Component/Process/Process.php
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ public function wait(callable $callback = null)
if (null !== $callback) {
if (!$this->processPipes->haveReadSupport()) {
$this->stop(0);
throw new \LogicException('Pass the callback to the Process::start method or enableOutput to use a callback with Process::wait');
throw new \LogicException('Pass the callback to the "Process::start" method or call enableOutput to use a callback with "Process::wait"');
}
$this->callback = $this->buildCallback($callback);
}
Expand All @@ -429,6 +429,45 @@ public function wait(callable $callback = null)
return $this->exitcode;
}

/**
* Waits until the callback returns true.
*
* The callback receives the type of output (out or err) and some bytes
* from the output in real-time while writing the standard input to the process.
* It allows to have feedback from the independent process during execution.
*
* @param callable $callback
*
* @throws RuntimeException When process timed out
* @throws LogicException When process is not yet started
*/
public function waitUntil(callable $callback)
{
$this->requireProcessIsStarted(__FUNCTION__);
$this->updateStatus(false);

if (!$this->processPipes->haveReadSupport()) {
$this->stop(0);
throw new \LogicException('Pass the callback to the "Process::start" method or call enableOutput to use a callback with "Process::waitUntil".');
}
$callback = $this->buildCallback($callback);

$wait = true;
do {
$this->checkTimeout();
$running = '\\' === \DIRECTORY_SEPARATOR ? $this->isRunning() : $this->processPipes->areOpen();
$output = $this->processPipes->readAndWrite($running, '\\' !== \DIRECTORY_SEPARATOR || !$running);

foreach ($output as $type => $data) {
if (3 !== $type) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Nek- ! Happy New Year! I'm a bit late to the party but… could you please explain what this magic number is about? I thought possible values would be 1 or 2, but I didn't expect 3… any pointers? I'm trying to investigate this failure: https://ci.appveyor.com/project/fabpot/symfony/builds/21320382#L1210

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This issue is not related to this condition (or doesn't look related to me). Basically, if you extend the timeout for windows only, it may fix the issue. It looks like windows is slooooooow (not that slower on my computer though) to execute a process.

But I can't remember why I used $type !== 3, sorry.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure it has to do with Windows being slow, because if you look, there is the beginning of the output, and then the end of it, but the middle is missing 😅 :

First iteration output\n
One more iteration output\n

The code of the test is pretty straightforward, that's why I'm starting to suspect the code might be at fault, maybe here or in WindowsPipes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 seems to be the index of a pipe that will receive the last exit code, and indeed, does not seem to have much to do with the problem at hand because it is does not seem to be used on Windows.

$wait = !$callback(self::STDOUT === $type ? self::OUT : self::ERR, $data);
} elseif (!isset($this->fallbackStatus['signaled'])) {
$this->fallbackStatus['exitcode'] = (int) $data;
}
}
} while ($wait);
}

/**
* Returns the Pid (process identifier), if applicable.
*
Expand Down Expand Up @@ -1264,7 +1303,7 @@ protected function buildCallback(callable $callback = null)
if ($this->outputDisabled) {
return function ($type, $data) use ($callback) {
if (null !== $callback) {
\call_user_func($callback, $type, $data);
return \call_user_func($callback, $type, $data);
}
};
}
Expand All @@ -1279,7 +1318,7 @@ protected function buildCallback(callable $callback = null)
}

if (null !== $callback) {
\call_user_func($callback, $type, $data);
return \call_user_func($callback, $type, $data);
}
};
}
Expand Down
26 changes: 26 additions & 0 deletions src/Symfony/Component/Process/Tests/KillableProcessWithOutput.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

$outputs = array(
'First iteration output',
'Second iteration output',
'One more iteration output',
'This took more time',
'This one was sooooo slow',
);

$iterationTime = 10000;

foreach ($outputs as $output) {
usleep($iterationTime);
$iterationTime *= 10;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this create an extremely slow test?

Copy link
Contributor Author

@Nek- Nek- Jun 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test should end after 1s and 110ms of execution of this code. If a failure occurs, the timeout will be reached 60s after the start of the test.

The process is long to end (but ends, for "security" purpose) to simulate the running of a daemon. It is stop by the test.

Note: for some reason under windows it looks like this test takes 4s. No idea why and how. Any though about?

Windows PHPUnit output on test process:

1) Symfony\Component\Process\Tests\ProcessTest::testWaitUntilSpecificOutput
Failed asserting that 4.3028318881989 is less than 2.
C:\projects\symfony\src\Symfony\Component\Process\Tests\ProcessTest.php:155

Here is an ugly fix suggestion.

echo $output."\n";
}
27 changes: 27 additions & 0 deletions src/Symfony/Component/Process/Tests/ProcessTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,33 @@ public function testStopWithTimeoutIsActuallyWorking()
$this->assertLessThan(15, microtime(true) - $start);
}

public function testWaitUntilSpecificOutput()
{
$p = $this->getProcess(array(self::$phpBin, __DIR__.'/KillableProcessWithOutput.php'));
$p->start();

$start = microtime(true);

$completeOutput = '';
$p->waitUntil(function ($type, $output) use (&$completeOutput) {
$completeOutput .= $output;
if (false !== strpos($output, 'One more')) {
return true;
}

return false;
});
$p->stop();

if ('\\' === \DIRECTORY_SEPARATOR) {
// Windows is slower
$this->assertLessThan(15, microtime(true) - $start);
} else {
$this->assertLessThan(2, microtime(true) - $start);
}
$this->assertEquals("First iteration output\nSecond iteration output\nOne more iteration output\n", $completeOutput);
}

public function testAllOutputIsActuallyReadOnTermination()
{
// this code will result in a maximum of 2 reads of 8192 bytes by calling
Expand Down