Skip to content
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

Allow signal handling for Uv w/o 100% cpu usage #330

Open
wants to merge 2 commits into
base: 2.x
Choose a base branch
from

Conversation

duckboy81
Copy link

Ref #329

To be honest this could lend itself to its own Uv specific example file, but here's something that could work regardless of driver used.

Again, downside is that the end of code execution is not in the signal handling function, but sometime after the Loop::run() line.

@enumag
Copy link
Contributor

enumag commented Oct 23, 2020

Not a very good solution imo. Now the signal handler can't control the exit code.

Comment on lines 10 to 22
Loop::onSignal(SIGINT, function () {
print "Caught SIGINT, exiting..." . PHP_EOL;

// Check for a Uv driver
if (Loop::get() instanceof Amp\Loop\UvDriver) {

// Stop the loop
Loop::stop();

// Cannot exit out of a UvDriver loop here, can only stop the loop
return;
}

Copy link
Member

Choose a reason for hiding this comment

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

What happens if you cancel the signal watcher before existing? Did you look where exactly it hangs in probably an infinite loop causing the 100% CPU usage?

Suggested change
Loop::onSignal(SIGINT, function () {
print "Caught SIGINT, exiting..." . PHP_EOL;
// Check for a Uv driver
if (Loop::get() instanceof Amp\Loop\UvDriver) {
// Stop the loop
Loop::stop();
// Cannot exit out of a UvDriver loop here, can only stop the loop
return;
}
Loop::onSignal(SIGINT, function ($watcherId) {
print "Caught SIGINT, exiting..." . PHP_EOL;
Loop::cancel($watcherId);

Copy link
Author

Choose a reason for hiding this comment

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

Not sure where my comment ended up, so sorry if this is a double tap.

Just tried this code and it still seems to get stuck in a loop. I tried debugging it for a couple hours, but I didn't get anywhere. Xdebug seemed to dump the profiler data before it got stuck in the loop, which made me think it was a Uv lib problem. (trying without xdebug loaded results in the same problem)

Hmm, looks like it still gets stuck in the loop. I tried for a couple hours to find where the loop occurs, but I didn't get anywhere. It seemed as if it was happening in due to the libuv module (it was happening after the xdebug profiler closed out).

Co-authored-by: Niklas Keller <me@kelunik.com>
@kelunik
Copy link
Member

kelunik commented Oct 23, 2020

I can't reproduce 100% CPU usage, but I get a warning:

Press Ctrl+C to exit...
^CCaught SIGINT, exiting...
PHP Warning:  zend_signal: handler was replaced for signal (2) after startup in Unknown on line 0

@duckboy81
Copy link
Author

I can't reproduce 100% CPU usage, but I get a warning:

Press Ctrl+C to exit...
^CCaught SIGINT, exiting...
PHP Warning:  zend_signal: handler was replaced for signal (2) after startup in Unknown on line 0

Using uv as the driver? (I've currently got uv 0.2.4-3 if it matters)

@kelunik
Copy link
Member

kelunik commented Oct 23, 2020

Please try with the latest master of https://github.com/bwoebi/php-uv.

@duckboy81
Copy link
Author

Please try with the latest master of https://github.com/bwoebi/php-uv.

Alright, give me a hot minute to try it.

PS: Thanks for being so active, you guys are awesome.

@duckboy81
Copy link
Author

Please try with the latest master of https://github.com/bwoebi/php-uv.

Alright, give me a hot minute to try it.

PS: Thanks for being so active, you guys are awesome.

No luck. Same result.

I'm not sure this is worth putting too much work into unless someone else runs into the same issue. They can reference this conversation.

@kelunik
Copy link
Member

kelunik commented Oct 23, 2020

Which PHP version do you use?

@duckboy81
Copy link
Author

Using the following:

PHP 7.4.2 (cli) (built: Jan 21 2020 11:35:20) ( NTS )
Zend Engine v3.4.0, Copyright (c) Zend Technologies
with Zend OPcache v7.4.2, Copyright (c), by Zend Technologies

If I switch from uv to the native driver, no issues too.

@trowski
Copy link
Member

trowski commented Oct 24, 2020

Perhaps there's an issue in ext-uv when calling exit inside a watcher callback? Exit handling changed recently to throw an exception internally (not sure which PHP version this happened in).

@kelunik kelunik changed the base branch from master to 2.x December 18, 2022 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants