Skip to content

Commit ff89670

Browse files
mhdawsondanbev
authored andcommitted
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 PR-URL: #27085 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent b925379 commit ff89670

File tree

1 file changed

+9
-7
lines changed

1 file changed

+9
-7
lines changed

src/js_native_api_v8.cc

+9-7
Original file line numberDiff line numberDiff line change
@@ -227,24 +227,26 @@ class Reference : private Finalizer {
227227
// from one of Unwrap or napi_delete_reference.
228228
//
229229
// When it is called from Unwrap or napi_delete_reference we only
230-
// want to do the delete if the finalizer has already run,
230+
// want to do the delete if the finalizer has already run or
231+
// cannot have been queued to run (ie the reference count is > 0),
231232
// otherwise we may crash when the finalizer does run.
232-
// If the finalizer has not already run delay the delete until
233-
// the finalizer runs by not doing the delete
233+
// If the finalizer may have been queued and has not already run
234+
// delay the delete until the finalizer runs by not doing the delete
234235
// and setting _delete_self to true so that the finalizer will
235236
// delete it when it runs.
236237
//
237238
// The second way this is called is from
238239
// the finalizer and _delete_self is set. In this case we
239240
// know we need to do the deletion so just do it.
240241
static void Delete(Reference* reference) {
241-
if ((reference->_delete_self) || (reference->_finalize_ran)) {
242+
if ((reference->RefCount() != 0) ||
243+
(reference->_delete_self) ||
244+
(reference->_finalize_ran)) {
242245
delete reference;
243246
} else {
244-
// reduce the reference count to 0 and defer until
245-
// finalizer runs
247+
// defer until finalizer runs as
248+
// it may alread be queued
246249
reference->_delete_self = true;
247-
while (reference->Unref() != 0) {}
248250
}
249251
}
250252

0 commit comments

Comments
 (0)