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

Backport src: fix handle leaks #7711 #9014

Merged
merged 4 commits into from
Oct 11, 2016

Conversation

MylesBorins
Copy link
Contributor

This is a backport of #7711 to v4.x-staging

@bnoordhuis would you be able to review? The only conflict was in 5f5eb12, specifically around removing HandleScopes.

AFAIK I got everything right, but don't want to risk it.

@MylesBorins MylesBorins added v4.x lib / src Issues and PRs related to general changes in the lib or src directory. labels Oct 10, 2016
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. v4.x labels Oct 10, 2016
@MylesBorins
Copy link
Contributor Author

@bnoordhuis
Copy link
Member

Not sure what is up with the CI. It's saying some tests failed but when you click through, everything is green.

@jasnell
Copy link
Member

jasnell commented Oct 11, 2016

LGTM

@MylesBorins
Copy link
Contributor Author

@MylesBorins
Copy link
Contributor Author

single failure is expected (freebsd)

Fix handle leaks in Buffer::New() and Buffer::Copy() by creating the
handle scope before looking up the env with Environment::GetCurrent().

Environment::GetCurrent() calls v8::Isolate::GetCurrentContext(), which
creates a handle in the current scope, i.e., the scope created by the
caller of Buffer::New() or Buffer::Copy().

PR-URL: nodejs#7711
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Create a handle scope before performing a check that creates a handle,
otherwise the handle is leaked into the handle scope of the caller.

PR-URL: nodejs#7711
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Create a handle scope before performing a check that creates a handle,
otherwise the handle is leaked into the handle scope of the caller.

PR-URL: nodejs#7711
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
API function callbacks run inside an implicit HandleScope.  We don't
need to explicitly create one and in fact introduce some unnecessary
overhead when we do.

PR-URL: nodejs#7711
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
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++. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants