diff options
author | Michael Dawson <michael_dawson@ca.ibm.com> | 2019-04-04 10:24:21 -0400 |
---|---|---|
committer | Daniel Bevenius <daniel.bevenius@gmail.com> | 2019-04-09 05:45:54 +0200 |
commit | ff89670902042cc6a6d99596c7e2d34fd11ad6a2 (patch) | |
tree | 83e3e124b511c2ff197a3320d5cd8ef5e7d49f2c /src/js_native_api_v8.cc | |
parent | b925379f506714e942f49789b7eed7bc4232c7ee (diff) | |
download | android-node-v8-ff89670902042cc6a6d99596c7e2d34fd11ad6a2.tar.gz android-node-v8-ff89670902042cc6a6d99596c7e2d34fd11ad6a2.tar.bz2 android-node-v8-ff89670902042cc6a6d99596c7e2d34fd11ad6a2.zip |
n-api: reduce gc finalization stress
https://github.com/nodejs/node/pull/24494 fixed a crash
but resulted in increased stress on gc finalization. A leak
was reported in https://github.com/nodejs/node/issues/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: https://github.com/nodejs/node/pull/27085
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Diffstat (limited to 'src/js_native_api_v8.cc')
-rw-r--r-- | src/js_native_api_v8.cc | 16 |
1 files changed, 9 insertions, 7 deletions
diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index f6914e72dd..134ae992ec 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -227,10 +227,11 @@ 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. // @@ -238,13 +239,14 @@ class Reference : private Finalizer { // 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) {} } } |