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

[bug fix v8.x] timers: fix a bug in error handling #20497

Closed

Conversation

apapirovski
Copy link
Member

@apapirovski apapirovski commented May 3, 2018

This was fixed in master via a semver-major commit which can't be backported, at least not until we confirm that it doesn't cause unrelated issues.

When a timeout within a list of timeouts (that consists of only that specific timeout) throws during its execution, it's possible for the TimerWrap handle to become shared between both that list and an unref'd timeout created in the future. This fixes the bug by extending error handling in timeout execution to check for whether the current list is empty and if so do cleanup on it.

Fixes: #19970

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

When a timeout within a list of timeouts (that consists of only
that specific timeout) throws during its execution, it's possible
for the TimerWrap handle to become shared between both that list
and an unref'd timeout created in the future. This fixes the bug
by extending error handling in timeout execution to check for
whether the current list is empty and if so do cleanup on it.
@apapirovski apapirovski added the timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. label May 3, 2018
@nodejs-github-bot nodejs-github-bot added timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. v8.x labels May 3, 2018
@apapirovski
Copy link
Member Author

@apapirovski apapirovski added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 4, 2018
MylesBorins pushed a commit that referenced this pull request May 4, 2018
When a timeout within a list of timeouts (that consists of only
that specific timeout) throws during its execution, it's possible
for the TimerWrap handle to become shared between both that list
and an unref'd timeout created in the future. This fixes the bug
by extending error handling in timeout execution to check for
whether the current list is empty and if so do cleanup on it.

PR-URL: #20497
Fixes: #19970
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
@MylesBorins
Copy link
Contributor

landed in ff2bd60

@MylesBorins MylesBorins closed this May 4, 2018
@apapirovski apapirovski deleted the fix-v8.x-timers-bug branch May 4, 2018 20:39
MylesBorins pushed a commit that referenced this pull request May 15, 2018
When a timeout within a list of timeouts (that consists of only
that specific timeout) throws during its execution, it's possible
for the TimerWrap handle to become shared between both that list
and an unref'd timeout created in the future. This fixes the bug
by extending error handling in timeout execution to check for
whether the current list is empty and if so do cleanup on it.

PR-URL: #20497
Fixes: #19970
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
@MylesBorins MylesBorins mentioned this pull request May 15, 2018
MylesBorins pushed a commit that referenced this pull request May 15, 2018
When a timeout within a list of timeouts (that consists of only
that specific timeout) throws during its execution, it's possible
for the TimerWrap handle to become shared between both that list
and an unref'd timeout created in the future. This fixes the bug
by extending error handling in timeout execution to check for
whether the current list is empty and if so do cleanup on it.

PR-URL: #20497
Fixes: #19970
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
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.

5 participants