Skip to content

napi: reduce gc finalization stress #27085

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

Closed
mhdawson wants to merge 1 commit into from
Closed

napi: reduce gc finalization stress #27085

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
n-api: reduce gc finalization stress 
#24494 fixed a crash
but resulted in increased stress on gc finalization. A leak
was reported in #26667 which
we are still investigating. As part of this investigation I
realized we can optimize to reduce amount of deferred finalization.
Regardless of the root cause of the leak this should be a
good optimization. It also resolves the leak for the case being
reported in #26667. The OP in 26667 has confirmed that he can
still recreate the original problem that 24494 fixed and that
the fix still works with this optimization
  • Loading branch information
@mhdawson
mhdawson committed Apr 5, 2019
commit 881efee7b8ed2fc27f85d978805affa8f67fff52
16 changes: 9 additions & 7 deletions src/js_native_api_v8.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,24 +227,26 @@ class Reference : private Finalizer {
// from one of Unwrap or napi_delete_reference.
//
// When it is called from Unwrap or napi_delete_reference we only
// want to do the delete if the finalizer has already run,
// want to do the delete if the finalizer has already run or
// cannot have been queued to run (ie the reference count is > 0),
// otherwise we may crash when the finalizer does run.
// If the finalizer has not already run delay the delete until
// the finalizer runs by not doing the delete
// If the finalizer may have been queued and has not already run
// delay the delete until the finalizer runs by not doing the delete
// and setting _delete_self to true so that the finalizer will
// delete it when it runs.
//
// The second way this is called is from
// the finalizer and _delete_self is set. In this case we
// know we need to do the deletion so just do it.
static void Delete(Reference* reference) {
if ((reference->_delete_self) || (reference->_finalize_ran)) {
if ((reference->RefCount() != 0) ||
Comment thread
bnoordhuis marked this conversation as resolved.
Outdated
(reference->_delete_self) ||
(reference->_finalize_ran)) {
delete reference;
} else {
// reduce the reference count to 0 and defer until
// finalizer runs
// defer until finalizer runs as
// it may alread be queued
reference->_delete_self = true;
while (reference->Unref() != 0) {}
}
}

Expand Down