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

src: use BaseObjectPtr in StreamReq::Dispose #33102

Closed

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Apr 27, 2020

Allow the AsyncWrap to be properly detached.

Extracted from the [QUIC PR](nodejs#32379).

Signed-off-by: James M Snell <[email protected]>
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Apr 27, 2020
@jasnell jasnell requested review from addaleax and sam-github April 27, 2020 20:05
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 28, 2020

@jasnell jasnell added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 28, 2020
@jasnell
Copy link
Member Author

jasnell commented May 6, 2020

Landed in 1dc006e

@codebytere
Copy link
Member

@jasnell should this go back to v12.x? There are a handful of conflicts - I'll add the label but feel free to change it!

@addaleax
Copy link
Member

addaleax commented Jun 7, 2020

@codebytere The changes conflict with #32307, but that’s just cleanup, and ignoring the conflicts should be totally okay here (i.e. git cherry-pick --strategy=recursive --strategy-option=theirs 1dc006ef1990d4dcb67a9521bc643638aeba248b). Would you feel comfortable applying that or should I open a backport PR with that?

@codebytere
Copy link
Member

Ah ok! sgtm - I'll take it :D

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. c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants