diff options
author | Michael Dawson <mdawson@devrus.com> | 2018-11-16 21:42:31 +0000 |
---|---|---|
committer | Michael Dawson <michael_dawson@ca.ibm.com> | 2018-11-23 10:01:54 -0500 |
commit | d45e303ac67c027bbad709f465e479ac789302a7 (patch) | |
tree | 8d1a0c369f5a1c59f2cd0a5ff5f5b463fe5e9450 /src/js_native_api_v8.cc | |
parent | f85b43537d6dff4004081fcc81f8aa771efbe1e5 (diff) | |
download | android-node-v8-d45e303ac67c027bbad709f465e479ac789302a7.tar.gz android-node-v8-d45e303ac67c027bbad709f465e479ac789302a7.tar.bz2 android-node-v8-d45e303ac67c027bbad709f465e479ac789302a7.zip |
n-api: handle reference delete before finalize
Crashes were reported during finalization due to
the memory for a reference being deleted and the
finalizer running after the deletion.
This change ensures the deletion of the memory for
the reference only occurs after the finalizer has run.
Fixes: https://github.com/nodejs/node-addon-api/issues/393
PR-URL: https://github.com/nodejs/node/pull/24494
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Diffstat (limited to 'src/js_native_api_v8.cc')
-rw-r--r-- | src/js_native_api_v8.cc | 33 |
1 files changed, 28 insertions, 5 deletions
diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 144cfad8e4..6b8d3f9105 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -227,8 +227,29 @@ class Reference : private Finalizer { finalize_hint); } + // Delete is called in 2 ways. Either from the finalizer or + // 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, + // 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 + // 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) { - delete reference; + if ((reference->_delete_self) || (reference->_finalize_ran)) { + delete reference; + } else { + // reduce the reference count to 0 and defer until + // finalizer runs + reference->_delete_self = true; + while (reference->Unref() != 0) {} + } } uint32_t Ref() { @@ -268,9 +289,6 @@ class Reference : private Finalizer { Reference* reference = data.GetParameter(); reference->_persistent.Reset(); - // Check before calling the finalize callback, because the callback might - // delete it. - bool delete_self = reference->_delete_self; napi_env env = reference->_env; if (reference->_finalize_callback != nullptr) { @@ -281,8 +299,13 @@ class Reference : private Finalizer { reference->_finalize_hint)); } - if (delete_self) { + // this is safe because if a request to delete the reference + // is made in the finalize_callback it will defer deletion + // to this block and set _delete_self to true + if (reference->_delete_self) { Delete(reference); + } else { + reference->_finalize_ran = true; } } |