Skip to content

[Process] add signal method. #5457

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 2 commits into from

Conversation

boombatower
Copy link
Contributor

Pull request for issue #5454.

Perhaps a signal($signal) method instead of kill() since stop is already taking that role and looks like other issue already talking about sending SIGTERM vs SIGKILL and what not.

It might be a good idea to limit the signals you can send so you don't terminate using this function. Also probably a good idea to have a list of POSIX signals as constants.

Thoughts?

@pborreli
Copy link
Contributor

pborreli commented Sep 7, 2012

please add test.

*/
public function signal($signal)
{
return proc_terminate($this->process, $signal);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong indent.

Also what with: https://bugs.php.net/bug.php?id=39992 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't speak for not terminating child, but as I commented in issue the signal seems to be sent through to child.

@boombatower
Copy link
Contributor Author

Short of doing something like http://php.net/manual/en/function.pcntl-signal.php and listening for a signal in a PHP process I am not sure how to test this. Possibly depend on some system process that pics up signals, but that doesn't necessarily seem like a good idea.

@sstok
Copy link
Contributor

sstok commented Sep 7, 2012

Already proposed in #5391

@boombatower
Copy link
Contributor Author

Looks like they took my advice and used proc_terminate() so another package was not needed. Thanks for catching duplicate, but at least it was not a waste. :)

@boombatower boombatower closed this Sep 7, 2012
@romainneutron
Copy link
Contributor

yup thanks @boombatower :)

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.

5 participants