From 1391400eeee1b45d6af1a5dff00d7ce2320b9865 Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Thu, 22 Jun 2023 19:39:22 -0700 Subject: [PATCH] node-api: run finalizers directly from GC --- src/js_native_api.h | 10 +++ src/js_native_api_v8.cc | 130 ++++++++++++++++++++++++++++----------- src/js_native_api_v8.h | 32 ++++++++-- src/node_api.cc | 2 +- src/node_api_internals.h | 2 + 5 files changed, 134 insertions(+), 42 deletions(-) diff --git a/src/js_native_api.h b/src/js_native_api.h index 3aa0ee5c1c5c5c..eb3c139a9deb41 100644 --- a/src/js_native_api.h +++ b/src/js_native_api.h @@ -517,6 +517,16 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_add_finalizer(napi_env env, #endif // NAPI_VERSION >= 5 +#ifdef NAPI_EXPERIMENTAL + +NAPI_EXTERN napi_status NAPI_CDECL +node_api_post_finalizer(napi_env env, + napi_finalize finalize_cb, + void* finalize_data, + void* finalize_hint); + +#endif // NAPI_EXPERIMENTAL + #if NAPI_VERSION >= 6 // BigInt diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 92f765eb0a99f9..23aaf3c70a87d4 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -57,8 +57,28 @@ (out) = v8::type::New((buffer), (byte_offset), (length)); \ } while (0) -namespace v8impl { +void napi_env__::InvokeFinalizerFromGC(v8impl::RefTracker* finalizer) { + if (module_api_version != NAPI_VERSION_EXPERIMENTAL) { + EnqueueFinalizer(finalizer); + } else { + // The experimental code calls finalizers immediately to release native + // objects as soon as possible, but it suspends use of JS from finalizer. + // If JS calls are needed, then the finalizer code must call + // node_api_post_finalizer. + if (last_error.error_code == napi_ok && last_exception.IsEmpty()) { + bool saved_suspend_call_into_js = suspend_call_into_js; + finalizer->Finalize(); + suspend_call_into_js = saved_suspend_call_into_js; + } else { + // The finalizers can be run in the middle of JS or C++ code. + // That code may be in an error state. In that case use the asynchronous + // finalizer. + EnqueueFinalizer(finalizer); + } + } +} +namespace v8impl { namespace { template @@ -604,28 +624,72 @@ void Finalizer::ResetFinalizer() { finalize_hint_ = nullptr; } -// Wrapper around v8impl::Persistent that implements reference counting. -RefBase::RefBase(napi_env env, - uint32_t initial_refcount, - Ownership ownership, - napi_finalize finalize_callback, - void* finalize_data, - void* finalize_hint) +TrackedFinalizer::TrackedFinalizer(napi_env env, + napi_finalize finalize_callback, + void* finalize_data, + void* finalize_hint) : Finalizer(env, finalize_callback, finalize_data, finalize_hint), - refcount_(initial_refcount), - ownership_(ownership) { + RefTracker() { Link(finalize_callback == nullptr ? &env->reflist : &env->finalizing_reflist); } -// When a RefBase is being deleted, it may have been queued to call its +TrackedFinalizer* TrackedFinalizer::New(napi_env env, + napi_finalize finalize_callback, + void* finalize_data, + void* finalize_hint) { + return new TrackedFinalizer( + env, finalize_callback, finalize_data, finalize_hint); +} + +// When a TrackedFinalizer is being deleted, it may have been queued to call its // finalizer. -RefBase::~RefBase() { +TrackedFinalizer::~TrackedFinalizer() { // Remove from the env's tracked list. Unlink(); // Try to remove the finalizer from the scheduled second pass callback. env_->DequeueFinalizer(this); } +void TrackedFinalizer::Finalize() { + FinalizeCore(/*deleteMe:*/ true); +} + +void TrackedFinalizer::FinalizeCore(bool deleteMe) { + // Swap out the field finalize_callback so that it can not be accidentally + // called more than once. + napi_finalize finalize_callback = finalize_callback_; + void* finalize_data = finalize_data_; + void* finalize_hint = finalize_hint_; + ResetFinalizer(); + + // Either the RefBase is going to be deleted in the finalize_callback or not, + // it should be removed from the tracked list. + Unlink(); + // 1. If the finalize_callback is present, it should either delete the + // derived RefBase, or set ownership with Ownership::kRuntime. + // 2. If the finalizer is not present, the derived RefBase can be deleted + // after the call. + if (finalize_callback != nullptr) { + env_->CallFinalizer(finalize_callback, finalize_data, finalize_hint); + // No access to `this` after finalize_callback is called. + } + + if (deleteMe) { + delete this; + } +} + +// Wrapper around v8impl::Persistent that implements reference counting. +RefBase::RefBase(napi_env env, + uint32_t initial_refcount, + Ownership ownership, + napi_finalize finalize_callback, + void* finalize_data, + void* finalize_hint) + : TrackedFinalizer(env, finalize_callback, finalize_data, finalize_hint), + refcount_(initial_refcount), + ownership_(ownership) {} + RefBase* RefBase::New(napi_env env, uint32_t initial_refcount, Ownership ownership, @@ -660,31 +724,9 @@ uint32_t RefBase::RefCount() { } void RefBase::Finalize() { - Ownership ownership = ownership_; - // Swap out the field finalize_callback so that it can not be accidentally - // called more than once. - napi_finalize finalize_callback = finalize_callback_; - void* finalize_data = finalize_data_; - void* finalize_hint = finalize_hint_; - ResetFinalizer(); - - // Either the RefBase is going to be deleted in the finalize_callback or not, - // it should be removed from the tracked list. - Unlink(); - // 1. If the finalize_callback is present, it should either delete the - // RefBase, or set ownership with Ownership::kRuntime. - // 2. If the finalizer is not present, the RefBase can be deleted after the - // call. - if (finalize_callback != nullptr) { - env_->CallFinalizer(finalize_callback, finalize_data, finalize_hint); - // No access to `this` after finalize_callback is called. - } - // If the RefBase is not Ownership::kRuntime, userland code should delete it. - // Now delete it if it is Ownership::kRuntime. - if (ownership == Ownership::kRuntime) { - delete this; - } + // Delete it if it is Ownership::kRuntime. + FinalizeCore(/*deleteMe:*/ ownership_ == Ownership::kRuntime); } template @@ -779,7 +821,7 @@ void Reference::WeakCallback(const v8::WeakCallbackInfo& data) { Reference* reference = data.GetParameter(); // The reference must be reset during the weak callback as the API protocol. reference->persistent_.Reset(); - reference->env_->EnqueueFinalizer(reference); + reference->env_->InvokeFinalizerFromGC(reference); } } // end of namespace v8impl @@ -3310,6 +3352,20 @@ napi_status NAPI_CDECL napi_add_finalizer(napi_env env, return napi_clear_last_error(env); } +#ifdef NAPI_EXPERIMENTAL + +napi_status NAPI_CDECL node_api_post_finalizer(napi_env env, + napi_finalize finalize_cb, + void* finalize_data, + void* finalize_hint) { + CHECK_ENV(env); + env->EnqueueFinalizer(v8impl::TrackedFinalizer::New( + env, finalize_cb, finalize_data, finalize_hint)); + return napi_clear_last_error(env); +} + +#endif + napi_status NAPI_CDECL napi_adjust_external_memory(napi_env env, int64_t change_in_bytes, int64_t* adjusted_value) { diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h index f7646778311bfd..ac9dc50f69c64f 100644 --- a/src/js_native_api_v8.h +++ b/src/js_native_api_v8.h @@ -68,7 +68,7 @@ struct napi_env__ { if (--refs == 0) DeleteMe(); } - virtual bool can_call_into_js() const { return true; } + virtual bool can_call_into_js() const { return !suspend_call_into_js; } static inline void HandleThrow(napi_env env, v8::Local value) { if (env->terminatedOrTerminating()) { @@ -102,9 +102,13 @@ struct napi_env__ { // Call finalizer immediately. virtual void CallFinalizer(napi_finalize cb, void* data, void* hint) { v8::HandleScope handle_scope(isolate); + v8::Context::Scope context_scope(context()); CallIntoModule([&](napi_env env) { cb(env, data, hint); }); } + // Invoke finalizer from V8 garbage collector. + void InvokeFinalizerFromGC(v8impl::RefTracker* finalizer); + // Enqueue the finalizer to the napi_env's own queue of the second pass // weak callback. // Implementation should drain the queue at the time it is safe to call @@ -148,6 +152,7 @@ struct napi_env__ { int refs = 1; void* instance_data = nullptr; int32_t module_api_version = NODE_API_DEFAULT_MODULE_API_VERSION; + bool suspend_call_into_js = false; protected: // Should not be deleted directly. Delete with `napi_env__::DeleteMe()` @@ -363,8 +368,28 @@ enum class Ownership { kUserland, }; -// Wrapper around Finalizer that implements reference counting. -class RefBase : public Finalizer, public RefTracker { +// Wrapper around Finalizer that can be tracked. +class TrackedFinalizer : public Finalizer, public RefTracker { + protected: + TrackedFinalizer(napi_env env, + napi_finalize finalize_callback, + void* finalize_data, + void* finalize_hint); + + public: + static TrackedFinalizer* New(napi_env env, + napi_finalize finalize_callback, + void* finalize_data, + void* finalize_hint); + ~TrackedFinalizer() override; + + protected: + void Finalize() override; + void FinalizeCore(bool deleteMe); +}; + +// Wrapper around TrackedFinalizer that implements reference counting. +class RefBase : public TrackedFinalizer { protected: RefBase(napi_env env, uint32_t initial_refcount, @@ -380,7 +405,6 @@ class RefBase : public Finalizer, public RefTracker { napi_finalize finalize_callback, void* finalize_data, void* finalize_hint); - virtual ~RefBase(); void* Data(); uint32_t Ref(); diff --git a/src/node_api.cc b/src/node_api.cc index 7537dc20b2bd82..1925f0fb886dc7 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -33,7 +33,7 @@ void node_napi_env__::DeleteMe() { } bool node_napi_env__::can_call_into_js() const { - return node_env()->can_call_into_js(); + return Super::can_call_into_js() && node_env()->can_call_into_js(); } void node_napi_env__::CallFinalizer(napi_finalize cb, void* data, void* hint) { diff --git a/src/node_api_internals.h b/src/node_api_internals.h index 25f6b291902024..d3b2b0c09b4c08 100644 --- a/src/node_api_internals.h +++ b/src/node_api_internals.h @@ -9,6 +9,8 @@ #include "util-inl.h" struct node_napi_env__ : public napi_env__ { + using Super = napi_env__; + node_napi_env__(v8::Local context, const std::string& module_filename, int32_t module_api_version);