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

napi: fixed memory leak in napi_async_init()/napi_async_destroy() #17714

Closed
wants to merge 1 commit into from
Closed

napi: fixed memory leak in napi_async_init()/napi_async_destroy() #17714

wants to merge 1 commit into from

Conversation

alnyan
Copy link

@alnyan alnyan commented Dec 17, 2017

This patch fixes memory leak related to async_context not being freed in napi_async_destroy() after being allocated in napi_async_init()

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

napi_async_init()/napi_async_destroy() in src/node_api.cc

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v4.x node-api Issues and PRs related to the Node-API. labels Dec 17, 2017
@aqrln aqrln added the async_hooks Issues and PRs related to the async hooks subsystem. label Dec 17, 2017
@aqrln
Copy link
Contributor

aqrln commented Dec 17, 2017

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

@aqrln aqrln added the fast-track PRs that do not need to wait for 48 hours to land. label Dec 17, 2017
@aqrln
Copy link
Contributor

aqrln commented Dec 17, 2017

I don't think this PR needs to wait for full 48 hours to land, so whoever agrees with this, feel free to land it right away (or feel free to remove the label if you don't).

aqrln pushed a commit to aqrln/node that referenced this pull request Dec 18, 2017
@aqrln
n-api: fix memory leak in napi_async_destroy()
69938b1 
PR-URL: nodejs#17714
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
aqrln pushed a commit that referenced this pull request Dec 18, 2017
@aqrln
n-api: fix memory leak in napi_async_destroy()
94e2951 
PR-URL: #17714
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@aqrln
Copy link
Contributor

aqrln commented Dec 18, 2017

Landed in 94e2951. Thanks for the contribution!

@aqrln aqrln closed this Dec 18, 2017
MylesBorins pushed a commit that referenced this pull request Jan 8, 2018
@MylesBorins
n-api: fix memory leak in napi_async_destroy()
e54a836 
PR-URL: #17714
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
@MylesBorins
n-api: fix memory leak in napi_async_destroy()
79a2833 
PR-URL: #17714
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jan 10, 2018
Merged
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Apr 12, 2018
n-api: fix memory leak in napi_async_destroy()
bba1442 
PR-URL: nodejs#17714
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Apr 16, 2018
n-api: fix memory leak in napi_async_destroy()
825468f 
PR-URL: nodejs#17714
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 16, 2018
@MylesBorins
n-api: fix memory leak in napi_async_destroy()
65ea7ab 
Backport-PR-URL: #19447
PR-URL: #17714
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Apr 16, 2018
Merged
MylesBorins pushed a commit that referenced this pull request May 1, 2018
@MylesBorins
n-api: fix memory leak in napi_async_destroy()
a744535 
Backport-PR-URL: #19265
PR-URL: #17714
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request May 2, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aqrln aqrln aqrln approved these changes

@cjihrig cjihrig cjihrig approved these changes

@TimothyGu TimothyGu TimothyGu approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
async_hooks Issues and PRs related to the async hooks subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. fast-track PRs that do not need to wait for 48 hours to land. node-api Issues and PRs related to the Node-API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@alnyan @aqrln @jasnell @addaleax @TimothyGu @cjihrig @AndreasMadsen @BethGriggs @nodejs-github-bot