Skip to content

Commit

Permalink
n-api: reduce gc finalization stress
Browse files Browse the repository at this point in the history
#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

PR-URL: #27085
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
mhdawson authored and danbev committed Apr 9, 2019
1 parent b925379 commit ff89670
Showing 1 changed file with 9 additions and 7 deletions.
16 changes: 9 additions & 7 deletions src/js_native_api_v8.cc
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) ||
(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

0 comments on commit ff89670

Please sign in to comment.