diff options
author | Gabriel Schulhof <gabriel.schulhof@intel.com> | 2019-09-05 18:38:19 -0700 |
---|---|---|
committer | Gabriel Schulhof <gabriel.schulhof@intel.com> | 2019-10-13 00:07:43 -0700 |
commit | 53ca0b9ae145c430842bf78e553e3b6cbd2823aa (patch) | |
tree | e7eb7372e2bd4cc16deeaa7310417f024231ffd6 /src/js_native_api_v8.h | |
parent | fce1a5198a89e8f62ea2d093e01971db46da9bf5 (diff) | |
download | android-node-v8-53ca0b9ae145c430842bf78e553e3b6cbd2823aa.tar.gz android-node-v8-53ca0b9ae145c430842bf78e553e3b6cbd2823aa.tar.bz2 android-node-v8-53ca0b9ae145c430842bf78e553e3b6cbd2823aa.zip |
src: render N-API weak callbacks as cleanup hooks
Since worker threads are complete Node.js environments, including the
ability to load native addons, and since those native addons can
allocate resources to be freed when objects go out of scope, and since,
upon worker thread exit, the engine does not invoke the weak callbacks
responsible for freeing resources which still have references, this
modification introduces tracking for weak references such that a list
of outstanding weak references is maintained. This list is traversed
during environment teardown. The callbacks for the remaining weak
references are called.
This change is also relevant for Node.js embedder scenarios, because in
those cases the process also outlives the `node::Environment` and
therefore weak callbacks should also be rendered as environment cleanup
hooks to ensure proper cleanup after native addons. This changes
introduces the means by which this can be accomplished.
A benchmark is included which measures the time it takes to execute the
weak reference callback for a given number of weak references.
Re: https://github.com/tc39/proposal-weakrefs/issues/125#issuecomment-535832130
PR-URL: https://github.com/nodejs/node/pull/28428
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Diffstat (limited to 'src/js_native_api_v8.h')
-rw-r--r-- | src/js_native_api_v8.h | 13 |
1 files changed, 13 insertions, 0 deletions
diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h index 506e693f82..2e0a7a1d6a 100644 --- a/src/js_native_api_v8.h +++ b/src/js_native_api_v8.h @@ -15,6 +15,13 @@ struct napi_env__ { CHECK_EQ(isolate, context->GetIsolate()); } virtual ~napi_env__() { + // First we must finalize those references that have `napi_finalizer` + // callbacks. The reason is that addons might store other references which + // they delete during their `napi_finalizer` callbacks. If we deleted such + // references here first, they would be doubly deleted when the + // `napi_finalizer` deleted them subsequently. + v8impl::RefTracker::FinalizeAll(&finalizing_reflist); + v8impl::RefTracker::FinalizeAll(&reflist); if (instance_data.finalize_cb != nullptr) { CallIntoModuleThrow([&](napi_env env) { instance_data.finalize_cb(env, instance_data.data, instance_data.hint); @@ -55,6 +62,12 @@ struct napi_env__ { } v8impl::Persistent<v8::Value> last_exception; + + // We store references in two different lists, depending on whether they have + // `napi_finalizer` callbacks, because we must first finalize the ones that + // have such a callback. See `~napi_env__()` above for details. + v8impl::RefTracker::RefList reflist; + v8impl::RefTracker::RefList finalizing_reflist; napi_extended_error_info last_error; int open_handle_scopes = 0; int open_callback_scopes = 0; |