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

[v10.x] async_hooks: avoid double-destroy HTTPParser #27986

Conversation

Flarna
Copy link
Member

@Flarna Flarna commented May 30, 2019

Avoid that destroy hook is invoked twice - once via Parser::Free() and once again via Parser::Reinitialize() by clearing the async_id in EmitDestroy().

Partial backport of #27477, a full backport would require also #25094 which has a dont-land-on-v10.x label on it.

Fixes: #26961

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

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. v10.x labels May 30, 2019
Avoid that destroy hook is invoked twice - once via `Parser::Free()`
and once again via `Parser::Reinitialize()` by clearing the async_id
in `EmitDestroy()`.

Partial backport of nodejs#27477, a full
backport would require also nodejs#25094
which has a dont-land-on-v10.x label on it.

Fixes: nodejs#26961
@Flarna Flarna force-pushed the async-hooks-double-destroy branch from 85e9958 to 35283e7 Compare May 30, 2019 22:19
@Flarna Flarna changed the title [v10.x] backport avoid double-destroy HTTPParser [v10.x] async_hooks: avoid double-destroy HTTPParser May 31, 2019
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

@BethGriggs BethGriggs added the review wanted PRs that need reviews. label Jun 7, 2019
@nodejs-github-bot
Copy link
Collaborator

BethGriggs pushed a commit that referenced this pull request Jun 28, 2019
Avoid that destroy hook is invoked twice - once via `Parser::Free()`
and once again via `Parser::Reinitialize()` by clearing the async_id
in `EmitDestroy()`.

Partial backport of #27477, a full
backport would require also #25094
which has a dont-land-on-v10.x label on it.

Fixes: #26961

Backport-PR-URL: #27986
PR-URL: #27477
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@BethGriggs
Copy link
Member

Landed on v10.x-staging

@BethGriggs BethGriggs closed this Jun 28, 2019
@Flarna Flarna deleted the async-hooks-double-destroy branch June 28, 2019 13:48
BethGriggs pushed a commit that referenced this pull request Jul 17, 2019
Avoid that destroy hook is invoked twice - once via `Parser::Free()`
and once again via `Parser::Reinitialize()` by clearing the async_id
in `EmitDestroy()`.

Partial backport of #27477, a full
backport would require also #25094
which has a dont-land-on-v10.x label on it.

Fixes: #26961

Backport-PR-URL: #27986
PR-URL: #27477
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. review wanted PRs that need reviews.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants