Skip to content

Conversation

apapirovski
Copy link
Contributor

When timers.refresh() is called inside a callback, the timer would incorrectly end up unrefed and thus not keep the event loop alive.

Fixes: #26642

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added the timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. label Mar 17, 2019
@apapirovski
Copy link
Contributor Author

@antsmartian antsmartian added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 18, 2019
When `timers.refresh()` is called inside a callback, the timer would
incorrectly end up unrefed and thus not keep the event loop alive.
@apapirovski apapirovski force-pushed the fix-timers-refresh-in-callback branch from 239c9ae to a70719a Compare March 19, 2019 05:42
@apapirovski
Copy link
Contributor Author

CI: https://ci.nodejs.org/job/node-test-pull-request/21655/ (had to rebase significantly)

@antsmartian
Copy link
Contributor

@danbev
Copy link
Contributor

danbev commented Mar 20, 2019

Landed in 4306300.

@danbev danbev closed this Mar 20, 2019
danbev pushed a commit that referenced this pull request Mar 20, 2019
When `timers.refresh()` is called inside a callback, the timer would
incorrectly end up unrefed and thus not keep the event loop alive.

PR-URL: #26721
Fixes: #26642
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Weijia Wang <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
targos pushed a commit that referenced this pull request Mar 27, 2019
When `timers.refresh()` is called inside a callback, the timer would
incorrectly end up unrefed and thus not keep the event loop alive.

PR-URL: #26721
Fixes: #26642
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Weijia Wang <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
zero1five added a commit to zero1five/node that referenced this pull request Jun 13, 2019
Add an or option for put refresh back to work. nodejs#26721 one reason it
can't be overridden it only works in the callback of the current
timer(before `finally`).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v11.11.0 timeout.refresh() does not seem to prevent event loop from exiting

9 participants