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.

Sign up for GitHub

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

Jump to bottom

src: delete BaseObjectWeakPtr data when pointee is gone #32393

Closed
wants to merge 1 commit into from
Closed

src: delete BaseObjectWeakPtr data when pointee is gone #32393

wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

Fix the condition for deleting the underlying data pointed to by
a BaseObjectWeakPtr, which erroneously skipped that deletion
when ptr->get() was nullptr. This fixes a memory leak reported
by some of the tests.

Refs: #30374 (comment)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@addaleax
src: delete BaseObjectWeakPtr data when pointee is gone
e6f450e 
Fix the condition for deleting the underlying data pointed to by
a `BaseObjectWeakPtr`, which erroneously skipped that deletion
when `ptr->get()` was `nullptr`. This fixes a memory leak reported
by some of the tests.

Refs: nodejs#30374 (comment)
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Mar 20, 2020
This was referenced Mar 20, 2020
Closed
Closed
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/29956/

@mmarchini
Copy link
Contributor

Can confirm, ASAN is happy with this!

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 21, 2020 •
edited by addaleax
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/29964/ (:yellow_heart:)

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 21, 2020
@mmarchini
Copy link
Contributor

Landed in 668bc11

@mmarchini mmarchini closed this Mar 23, 2020
mmarchini pushed a commit that referenced this pull request Mar 23, 2020
@addaleax @mmarchini
src: delete BaseObjectWeakPtr data when pointee is gone
668bc11 
Fix the condition for deleting the underlying data pointed to by
a `BaseObjectWeakPtr`, which erroneously skipped that deletion
when `ptr->get()` was `nullptr`. This fixes a memory leak reported
by some of the tests.

Refs: #30374 (comment)

PR-URL: #32393
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@addaleax addaleax deleted the base-object-weak-delete branch March 23, 2020 19:21
MylesBorins pushed a commit that referenced this pull request Mar 24, 2020
@addaleax @MylesBorins
src: delete BaseObjectWeakPtr data when pointee is gone
4ef844d 
Fix the condition for deleting the underlying data pointed to by
a `BaseObjectWeakPtr`, which erroneously skipped that deletion
when `ptr->get()` was `nullptr`. This fixes a memory leak reported
by some of the tests.

Refs: #30374 (comment)

PR-URL: #32393
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Mar 24, 2020
Merged
MylesBorins pushed a commit that referenced this pull request Mar 24, 2020
@addaleax @MylesBorins
src: delete BaseObjectWeakPtr data when pointee is gone
6f06cf0 
Fix the condition for deleting the underlying data pointed to by
a `BaseObjectWeakPtr`, which erroneously skipped that deletion
when `ptr->get()` was `nullptr`. This fixes a memory leak reported
by some of the tests.

Refs: #30374 (comment)

PR-URL: #32393
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit to addaleax/node that referenced this pull request Apr 1, 2020
@addaleax @MylesBorins
src: delete BaseObjectWeakPtr data when pointee is gone
d85cb07 
Fix the condition for deleting the underlying data pointed to by
a `BaseObjectWeakPtr`, which erroneously skipped that deletion
when `ptr->get()` was `nullptr`. This fixes a memory leak reported
by some of the tests.

Refs: nodejs#30374 (comment)

PR-URL: nodejs#32393
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 1, 2020
@addaleax @MylesBorins
src: delete BaseObjectWeakPtr data when pointee is gone
fbd5be6 
Fix the condition for deleting the underlying data pointed to by
a `BaseObjectWeakPtr`, which erroneously skipped that deletion
when `ptr->get()` was `nullptr`. This fixes a memory leak reported
by some of the tests.

Refs: #30374 (comment)

Backport-PR-URL: #32301
PR-URL: #32393
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@codebytere codebytere mentioned this pull request Apr 3, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmarchini mmarchini mmarchini approved these changes

@cjihrig cjihrig cjihrig approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@addaleax @nodejs-github-bot @mmarchini @jasnell @cjihrig