Skip to content

[Process] Fix #8739 #8740

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

Closed
wants to merge 4 commits into from
Closed

Conversation

romainneutron
Copy link
Contributor

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

This adds a fix to #8739. Whenever a call is done to to any non-blocking methods (Process::isRunning, Process::isStopped, Process::isTerminated, Process::getStatus, Process::getPid...), buffers are read, and callback executed.

Such code will now work :

$process->start(function ($type, $data) {
    echo $data;
});

while ($process->isRunning()) {
    // some stuff
    // callback is executed
}

@fabpot
Copy link
Member

fabpot commented Aug 13, 2013

In the ticket, you say that the issue is also present in 2.1. Why not fix 2.2 instead then?

@@ -254,7 +271,7 @@ public function testStatus()

public function testStop()
{
$process = $this->getProcess('php -r "while (true) {}"');
Copy link
Member

Choose a reason for hiding this comment

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

Why change this code?

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 is part of this commit romainneutron@923fc58

On my computer, zombie processes might have remain in case of unit test failure.
It happend that some infinite loops were running and slowing down my system. A ps call alerted me that many (~100) php -r "while (true) {}" were running after half an hour debugging things.

I propose to change these. As tests pass and nothing changed in the way we test things, I think it's a better idea to have these commands instead of the infinite loops for symfony/process developers.

@romainneutron
Copy link
Contributor Author

sure, boss

@romainneutron
Copy link
Contributor Author

Closed in favor of #8741

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants