Skip to content

loop: add pending work to loop-alive check#3466

Merged
vtjnash merged 1 commit into
libuv:v1.xfrom
vtjnash:jn/loop-alive
Feb 13, 2022
Merged

loop: add pending work to loop-alive check#3466
vtjnash merged 1 commit into
libuv:v1.xfrom
vtjnash:jn/loop-alive

Conversation

@vtjnash

@vtjnash vtjnash commented Feb 9, 2022

Copy link
Copy Markdown
Member

Pending work may be either (on any platform) pending_queue callbacks or
(on unix) watcher_queue handles to add to the io poll object.
Previously, we might have gotten somewhat stuck if the user caused an
event to be added to one of these in the idle or prepare callbacks, or
was embedding libuv.

Refs: #3234
Refs: #3101
Refs: #3308

@bnoordhuis bnoordhuis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM but with a suggestion to tweak the boolean logic for comprehensibility.

Comment thread src/unix/core.c Outdated
Pending work may be either (on any platform) pending_queue callbacks or
(on unix) watcher_queue handles to add to the io poll object.
Previously, we might have gotten somewhat stuck if the user caused an
event to be added to one of these in the idle or prepare callbacks, or
was embedding libuv.

Refs: libuv#3234
Refs: libuv#3101
Refs: libuv#3308
@vtjnash

vtjnash commented Feb 10, 2022

Copy link
Copy Markdown
Member Author

I changed the uv__loop_alive to a comment, although I was expecting the compiler was likely to remove the duplication previously.

@vtjnash vtjnash merged commit 939a056 into libuv:v1.x Feb 13, 2022
@vtjnash vtjnash deleted the jn/loop-alive branch February 13, 2022 05:12
@vtjnash

vtjnash commented Feb 13, 2022

Copy link
Copy Markdown
Member Author

I am somewhat suspicious that this may have broken the fork_signal_to_child test, as UV_RUN_ONCE is a very risky thing to test, and we always initialize the loop with pending work.

vtjnash added a commit to vtjnash/libuv that referenced this pull request Feb 14, 2022
I created `uv__backend_timeout` to be used internally for this reason,
then forgot to use it, resulting in flaky tests and excessive trips
around the uv_run loop.

Fix libuv#3472
vtjnash added a commit that referenced this pull request Feb 14, 2022
I created `uv__backend_timeout` to be used internally for this reason,
then forgot to use it, resulting in flaky tests and excessive trips
around the uv_run loop.

Fix #3472
bnoordhuis added a commit to bnoordhuis/libuv that referenced this pull request Apr 2, 2022
This reverts commit 939a056.
This reverts commit cc7dbaa.

Reverted for causing a regression in the Node.js test suite.

Also revert "fix oopsie from libuv#3466 (libuv#3475)".

Refs: nodejs/node#42340
JeffroMF pushed a commit to JeffroMF/libuv that referenced this pull request May 16, 2022
Pending work may be either (on any platform) pending_queue callbacks or
(on unix) watcher_queue handles to add to the io poll object.
Previously, we might have gotten somewhat stuck if the user caused an
event to be added to one of these in the idle or prepare callbacks, or
was embedding libuv.

Refs: libuv#3234
Refs: libuv#3101
Refs: libuv#3308
JeffroMF pushed a commit to JeffroMF/libuv that referenced this pull request May 16, 2022
I created `uv__backend_timeout` to be used internally for this reason,
then forgot to use it, resulting in flaky tests and excessive trips
around the uv_run loop.

Fix libuv#3472
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