summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--node.gyp1
-rw-r--r--src/js_native_api_v8.cc297
-rw-r--r--src/js_native_api_v8.h153
-rw-r--r--src/node_api.cc97
-rw-r--r--src/node_api_internals.h4
-rw-r--r--test/cctest/test_js_native_api_v8.cc102
-rw-r--r--test/js-native-api/6_object_wrap/6_object_wrap.cc63
-rw-r--r--test/js-native-api/6_object_wrap/test-object-wrap-ref.js13
8 files changed, 317 insertions, 413 deletions
diff --git a/node.gyp b/node.gyp
index 448cb8a8c7..4ef15c6248 100644
--- a/node.gyp
+++ b/node.gyp
@@ -1003,7 +1003,6 @@
'test/cctest/test_base_object_ptr.cc',
'test/cctest/test_node_postmortem_metadata.cc',
'test/cctest/test_environment.cc',
- 'test/cctest/test_js_native_api_v8.cc',
'test/cctest/test_linked_binding.cc',
'test/cctest/test_node_api.cc',
'test/cctest/test_per_process.cc',
diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc
index bf38108c6b..f92e3a0121 100644
--- a/src/js_native_api_v8.cc
+++ b/src/js_native_api_v8.cc
@@ -218,7 +218,12 @@ inline napi_status Unwrap(napi_env env,
if (action == RemoveWrap) {
CHECK(obj->DeletePrivate(context, NAPI_PRIVATE_KEY(context, wrapper))
.FromJust());
- Reference::Delete(reference);
+ if (reference->ownership() == Ownership::kUserland) {
+ // When the wrap is been removed, the finalizer should be reset.
+ reference->ResetFinalizer();
+ } else {
+ delete reference;
+ }
}
return GET_RETURN_STATUS(env);
@@ -245,7 +250,8 @@ class CallbackBundle {
bundle->env = env;
v8::Local<v8::Value> cbdata = v8::External::New(env->isolate, bundle);
- Reference::New(env, cbdata, 0, true, Delete, bundle, nullptr);
+ Reference::New(
+ env, cbdata, 0, Ownership::kRuntime, Delete, bundle, nullptr);
return cbdata;
}
napi_env env; // Necessary to invoke C++ NAPI callback
@@ -429,8 +435,13 @@ inline napi_status Wrap(napi_env env,
// before then, then the finalize callback will never be invoked.)
// Therefore a finalize callback is required when returning a reference.
CHECK_ARG(env, finalize_cb);
- reference = v8impl::Reference::New(
- env, obj, 0, false, finalize_cb, native_object, finalize_hint);
+ reference = v8impl::Reference::New(env,
+ obj,
+ 0,
+ v8impl::Ownership::kUserland,
+ finalize_cb,
+ native_object,
+ finalize_hint);
*result = reinterpret_cast<napi_ref>(reference);
} else {
// Create a self-deleting reference.
@@ -438,7 +449,7 @@ inline napi_status Wrap(napi_env env,
env,
obj,
0,
- true,
+ v8impl::Ownership::kRuntime,
finalize_cb,
native_object,
finalize_cb == nullptr ? nullptr : finalize_hint);
@@ -456,166 +467,144 @@ inline napi_status Wrap(napi_env env,
} // end of anonymous namespace
+void Finalizer::ResetFinalizer() {
+ finalize_callback_ = nullptr;
+ finalize_data_ = nullptr;
+ finalize_hint_ = nullptr;
+}
+
// Wrapper around v8impl::Persistent that implements reference counting.
RefBase::RefBase(napi_env env,
uint32_t initial_refcount,
- bool delete_self,
+ Ownership ownership,
napi_finalize finalize_callback,
void* finalize_data,
void* finalize_hint)
: Finalizer(env, finalize_callback, finalize_data, finalize_hint),
- _refcount(initial_refcount),
- _delete_self(delete_self) {
+ refcount_(initial_refcount),
+ ownership_(ownership) {
Link(finalize_callback == nullptr ? &env->reflist : &env->finalizing_reflist);
}
+// When a RefBase is being deleted, it may have been queued to call its
+// finalizer.
+RefBase::~RefBase() {
+ // Remove from the env's tracked list.
+ Unlink();
+ // Try to remove the finalizer from the scheduled second pass callback.
+ env_->DequeueFinalizer(this);
+}
+
RefBase* RefBase::New(napi_env env,
uint32_t initial_refcount,
- bool delete_self,
+ Ownership ownership,
napi_finalize finalize_callback,
void* finalize_data,
void* finalize_hint) {
return new RefBase(env,
initial_refcount,
- delete_self,
+ ownership,
finalize_callback,
finalize_data,
finalize_hint);
}
-RefBase::~RefBase() {
- Unlink();
-}
-
void* RefBase::Data() {
- return _finalize_data;
-}
-
-// 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 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 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.
-//
-// 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.
-void RefBase::Delete(RefBase* reference) {
- if ((reference->RefCount() != 0) || (reference->_delete_self) ||
- (reference->_finalize_ran)) {
- delete reference;
- } else {
- // defer until finalizer runs as
- // it may already be queued
- reference->_delete_self = true;
- }
+ return finalize_data_;
}
uint32_t RefBase::Ref() {
- return ++_refcount;
+ return ++refcount_;
}
uint32_t RefBase::Unref() {
- if (_refcount == 0) {
+ if (refcount_ == 0) {
return 0;
}
- return --_refcount;
+ return --refcount_;
}
uint32_t RefBase::RefCount() {
- return _refcount;
-}
-
-void RefBase::Finalize(bool is_env_teardown) {
- // In addition to being called during environment teardown, this method is
- // also the entry point for the garbage collector. During environment
- // teardown we have to remove the garbage collector's reference to this
- // method so that, if, as part of the user's callback, JS gets executed,
- // resulting in a garbage collection pass, this method is not re-entered as
- // part of that pass, because that'll cause a double free (as seen in
- // https://github.com/nodejs/node/issues/37236).
- //
- // Since this class does not have access to the V8 persistent reference,
- // this method is overridden in the `Reference` class below. Therein the
- // weak callback is removed, ensuring that the garbage collector does not
- // re-enter this method, and the method chains up to continue the process of
- // environment-teardown-induced finalization.
-
- // During environment teardown we have to convert a strong reference to
- // a weak reference to force the deferring behavior if the user's finalizer
- // happens to delete this reference so that the code in this function that
- // follows the call to the user's finalizer may safely access variables from
- // this instance.
- if (is_env_teardown && RefCount() > 0) _refcount = 0;
-
- if (_finalize_callback != nullptr) {
- // This ensures that we never call the finalizer twice.
- napi_finalize fini = _finalize_callback;
- _finalize_callback = nullptr;
- _env->CallFinalizer(fini, _finalize_data, _finalize_hint);
- }
-
- // 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 (_delete_self || is_env_teardown) {
- Delete(this);
- } else {
- _finalize_ran = true;
+ return 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;
}
}
template <typename... Args>
Reference::Reference(napi_env env, v8::Local<v8::Value> value, Args&&... args)
: RefBase(env, std::forward<Args>(args)...),
- _persistent(env->isolate, value),
- _secondPassParameter(new SecondPassCallParameterRef(this)),
- _secondPassScheduled(false) {
+ persistent_(env->isolate, value) {
if (RefCount() == 0) {
SetWeak();
}
}
+Reference::~Reference() {
+ // Reset the handle. And no weak callback will be invoked.
+ persistent_.Reset();
+}
+
Reference* Reference::New(napi_env env,
v8::Local<v8::Value> value,
uint32_t initial_refcount,
- bool delete_self,
+ Ownership ownership,
napi_finalize finalize_callback,
void* finalize_data,
void* finalize_hint) {
return new Reference(env,
value,
initial_refcount,
- delete_self,
+ ownership,
finalize_callback,
finalize_data,
finalize_hint);
}
-Reference::~Reference() {
- // If the second pass callback is scheduled, it will delete the
- // parameter passed to it, otherwise it will never be scheduled
- // and we need to delete it here.
- if (!_secondPassScheduled) {
- delete _secondPassParameter;
- }
-}
-
uint32_t Reference::Ref() {
+ // When the persistent_ is cleared in the WeakCallback, and a second pass
+ // callback is pending, return 0 unconditionally.
+ if (persistent_.IsEmpty()) {
+ return 0;
+ }
uint32_t refcount = RefBase::Ref();
if (refcount == 1) {
- ClearWeak();
+ persistent_.ClearWeak();
}
return refcount;
}
uint32_t Reference::Unref() {
+ // When the persistent_ is cleared in the WeakCallback, and a second pass
+ // callback is pending, return 0 unconditionally.
+ if (persistent_.IsEmpty()) {
+ return 0;
+ }
uint32_t old_refcount = RefCount();
uint32_t refcount = RefBase::Unref();
if (old_refcount == 1 && refcount == 0) {
@@ -625,99 +614,36 @@ uint32_t Reference::Unref() {
}
v8::Local<v8::Value> Reference::Get() {
- if (_persistent.IsEmpty()) {
+ if (persistent_.IsEmpty()) {
return v8::Local<v8::Value>();
} else {
- return v8::Local<v8::Value>::New(_env->isolate, _persistent);
+ return v8::Local<v8::Value>::New(env_->isolate, persistent_);
}
}
-void Reference::Finalize(bool is_env_teardown) {
- // During env teardown, `~napi_env()` alone is responsible for finalizing.
- // Thus, we don't want any stray gc passes to trigger a second call to
- // `RefBase::Finalize()`. ClearWeak will ensure that even if the
- // gc is in progress no Finalization will be run for this Reference
- // by the gc.
- if (is_env_teardown) {
- ClearWeak();
- }
+void Reference::Finalize() {
+ // Unconditionally reset the persistent handle so that no weak callback will
+ // be invoked again.
+ persistent_.Reset();
// Chain up to perform the rest of the finalization.
- RefBase::Finalize(is_env_teardown);
-}
-
-// ClearWeak is marking the Reference so that the gc should not
-// collect it, but it is possible that a second pass callback
-// may have been scheduled already if we are in shutdown. We clear
-// the secondPassParameter so that even if it has been
-// scheduled no Finalization will be run.
-void Reference::ClearWeak() {
- if (!_persistent.IsEmpty()) {
- _persistent.ClearWeak();
- }
- if (_secondPassParameter != nullptr) {
- *_secondPassParameter = nullptr;
- }
+ RefBase::Finalize();
}
// Mark the reference as weak and eligible for collection
// by the gc.
void Reference::SetWeak() {
- if (_secondPassParameter == nullptr) {
- // This means that the Reference has already been processed
- // by the second pass callback, so its already been Finalized, do
- // nothing
- return;
- }
- _persistent.SetWeak(
- _secondPassParameter, FinalizeCallback, v8::WeakCallbackType::kParameter);
- *_secondPassParameter = this;
+ persistent_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
}
// The N-API finalizer callback may make calls into the engine. V8's heap is
// not in a consistent state during the weak callback, and therefore it does
-// not support calls back into it. However, it provides a mechanism for adding
-// a finalizer which may make calls back into the engine by allowing us to
-// attach such a second-pass finalizer from the first pass finalizer. Thus,
-// we do that here to ensure that the N-API finalizer callback is free to call
-// into the engine.
-void Reference::FinalizeCallback(
- const v8::WeakCallbackInfo<SecondPassCallParameterRef>& data) {
- SecondPassCallParameterRef* parameter = data.GetParameter();
- Reference* reference = *parameter;
- if (reference == nullptr) {
- return;
- }
-
- // The reference must be reset during the first pass.
- reference->_persistent.Reset();
- // Mark the parameter not delete-able until the second pass callback is
- // invoked.
- reference->_secondPassScheduled = true;
-
- data.SetSecondPassCallback(SecondPassCallback);
-}
-
-// Second pass callbacks are scheduled with platform tasks. At env teardown,
-// the tasks may have already be scheduled and we are unable to cancel the
-// second pass callback task. We have to make sure that parameter is kept
-// alive until the second pass callback is been invoked. In order to do
-// this and still allow our code to Finalize/delete the Reference during
-// shutdown we have to use a separately allocated parameter instead
-// of a parameter within the Reference object itself. This is what
-// is stored in _secondPassParameter and it is allocated in the
-// constructor for the Reference.
-void Reference::SecondPassCallback(
- const v8::WeakCallbackInfo<SecondPassCallParameterRef>& data) {
- SecondPassCallParameterRef* parameter = data.GetParameter();
- Reference* reference = *parameter;
- delete parameter;
- if (reference == nullptr) {
- // the reference itself has already been deleted so nothing to do
- return;
- }
- reference->_secondPassParameter = nullptr;
- reference->Finalize();
+// not support calls back into it. Enqueue the invocation of the finalizer.
+void Reference::WeakCallback(const v8::WeakCallbackInfo<Reference>& 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);
}
} // end of namespace v8impl
@@ -2391,8 +2317,13 @@ napi_status NAPI_CDECL napi_create_external(napi_env env,
if (finalize_cb) {
// The Reference object will delete itself after invoking the finalizer
// callback.
- v8impl::Reference::New(
- env, external_value, 0, true, finalize_cb, data, finalize_hint);
+ v8impl::Reference::New(env,
+ external_value,
+ 0,
+ v8impl::Ownership::kRuntime,
+ finalize_cb,
+ data,
+ finalize_hint);
}
*result = v8impl::JsValueFromV8LocalValue(external_value);
@@ -2501,8 +2432,8 @@ napi_status NAPI_CDECL napi_create_reference(napi_env env,
return napi_set_last_error(env, napi_invalid_arg);
}
- v8impl::Reference* reference =
- v8impl::Reference::New(env, v8_value, initial_refcount, false);
+ v8impl::Reference* reference = v8impl::Reference::New(
+ env, v8_value, initial_refcount, v8impl::Ownership::kUserland);
*result = reinterpret_cast<napi_ref>(reference);
return napi_clear_last_error(env);
@@ -2516,7 +2447,7 @@ napi_status NAPI_CDECL napi_delete_reference(napi_env env, napi_ref ref) {
CHECK_ENV(env);
CHECK_ARG(env, ref);
- v8impl::Reference::Delete(reinterpret_cast<v8impl::Reference*>(ref));
+ delete reinterpret_cast<v8impl::Reference*>(ref);
return napi_clear_last_error(env);
}
@@ -3206,11 +3137,11 @@ napi_status NAPI_CDECL napi_set_instance_data(napi_env env,
if (old_data != nullptr) {
// Our contract so far has been to not finalize any old data there may be.
// So we simply delete it.
- v8impl::RefBase::Delete(old_data);
+ delete old_data;
}
- env->instance_data =
- v8impl::RefBase::New(env, 0, true, finalize_cb, data, finalize_hint);
+ env->instance_data = v8impl::RefBase::New(
+ env, 0, v8impl::Ownership::kRuntime, finalize_cb, data, finalize_hint);
return napi_clear_last_error(env);
}
diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h
index 766398744c..b3d0446cb5 100644
--- a/src/js_native_api_v8.h
+++ b/src/js_native_api_v8.h
@@ -4,7 +4,7 @@
#include "js_native_api_types.h"
#include "js_native_api_v8_internals.h"
-static napi_status napi_clear_last_error(napi_env env);
+inline napi_status napi_clear_last_error(napi_env env);
namespace v8impl {
@@ -12,7 +12,7 @@ class RefTracker {
public:
RefTracker() {}
virtual ~RefTracker() {}
- virtual void Finalize(bool isEnvTeardown) {}
+ virtual void Finalize() {}
typedef RefTracker RefList;
@@ -38,7 +38,7 @@ class RefTracker {
static void FinalizeAll(RefList* list) {
while (list->next_ != nullptr) {
- list->next_->Finalize(true);
+ list->next_->Finalize();
}
}
@@ -47,12 +47,12 @@ class RefTracker {
RefList* prev_ = nullptr;
};
+class Finalizer;
} // end of namespace v8impl
struct napi_env__ {
explicit napi_env__(v8::Local<v8::Context> context)
: isolate(context->GetIsolate()), context_persistent(isolate, context) {
- CHECK_EQ(isolate, context->GetIsolate());
napi_clear_last_error(this);
}
@@ -89,13 +89,26 @@ struct napi_env__ {
}
}
- // This should be overridden to schedule the finalization to a properiate
- // timing, like next tick of the event loop.
+ // Call finalizer immediately.
virtual void CallFinalizer(napi_finalize cb, void* data, void* hint) {
v8::HandleScope handle_scope(isolate);
CallIntoModule([&](napi_env env) { cb(env, data, hint); });
}
+ // 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
+ // into JavaScript.
+ virtual void EnqueueFinalizer(v8impl::RefTracker* finalizer) {
+ pending_finalizers.emplace(finalizer);
+ }
+
+ // Remove the finalizer from the scheduled second pass weak callback queue.
+ // The finalizer can be deleted after this call.
+ virtual void DequeueFinalizer(v8impl::RefTracker* finalizer) {
+ pending_finalizers.erase(finalizer);
+ }
+
virtual void DeleteMe() {
// First we must finalize those references that have `napi_finalizer`
// callbacks. The reason is that addons might store other references which
@@ -117,6 +130,8 @@ struct napi_env__ {
// have such a callback. See `~napi_env__()` above for details.
v8impl::RefTracker::RefList reflist;
v8impl::RefTracker::RefList finalizing_reflist;
+ // The invocation order of the finalizers is not determined.
+ std::unordered_set<v8impl::RefTracker*> pending_finalizers;
napi_extended_error_info last_error;
int open_handle_scopes = 0;
int open_callback_scopes = 0;
@@ -129,37 +144,8 @@ struct napi_env__ {
virtual ~napi_env__() = default;
};
-// This class is used to keep a napi_env live in a way that
-// is exception safe versus calling Ref/Unref directly
-class EnvRefHolder {
- public:
- explicit EnvRefHolder(napi_env env) : _env(env) { _env->Ref(); }
-
- explicit EnvRefHolder(const EnvRefHolder& other) : _env(other.env()) {
- _env->Ref();
- }
-
- EnvRefHolder(EnvRefHolder&& other) {
- _env = other._env;
- other._env = nullptr;
- }
-
- ~EnvRefHolder() {
- if (_env != nullptr) {
- _env->Unref();
- }
- }
-
- napi_env env(void) const { return _env; }
-
- private:
- napi_env _env;
-};
-
inline napi_status napi_clear_last_error(napi_env env) {
env->last_error.error_code = napi_ok;
-
- // TODO(boingoing): Should this be a callback?
env->last_error.engine_error_code = 0;
env->last_error.engine_reserved = nullptr;
env->last_error.error_message = nullptr;
@@ -306,49 +292,37 @@ inline v8::Local<v8::Value> V8LocalValueFromJsValue(napi_value v) {
// Adapter for napi_finalize callbacks.
class Finalizer {
- public:
- // Some Finalizers are run during shutdown when the napi_env is destroyed,
- // and some need to keep an explicit reference to the napi_env because they
- // are run independently.
- enum EnvReferenceMode { kNoEnvReference, kKeepEnvReference };
-
protected:
Finalizer(napi_env env,
napi_finalize finalize_callback,
void* finalize_data,
- void* finalize_hint,
- EnvReferenceMode refmode = kNoEnvReference)
- : _env(env),
- _finalize_callback(finalize_callback),
- _finalize_data(finalize_data),
- _finalize_hint(finalize_hint),
- _has_env_reference(refmode == kKeepEnvReference) {
- if (_has_env_reference) _env->Ref();
- }
+ void* finalize_hint)
+ : env_(env),
+ finalize_callback_(finalize_callback),
+ finalize_data_(finalize_data),
+ finalize_hint_(finalize_hint) {}
- ~Finalizer() {
- if (_has_env_reference) _env->Unref();
- }
+ virtual ~Finalizer() = default;
public:
static Finalizer* New(napi_env env,
napi_finalize finalize_callback = nullptr,
void* finalize_data = nullptr,
- void* finalize_hint = nullptr,
- EnvReferenceMode refmode = kNoEnvReference) {
- return new Finalizer(
- env, finalize_callback, finalize_data, finalize_hint, refmode);
+ void* finalize_hint = nullptr) {
+ return new Finalizer(env, finalize_callback, finalize_data, finalize_hint);
}
- static void Delete(Finalizer* finalizer) { delete finalizer; }
+ napi_finalize callback() { return finalize_callback_; }
+ void* data() { return finalize_data_; }
+ void* hint() { return finalize_hint_; }
+
+ void ResetFinalizer();
protected:
- napi_env _env;
- napi_finalize _finalize_callback;
- void* _finalize_data;
- void* _finalize_hint;
- bool _finalize_ran = false;
- bool _has_env_reference = false;
+ napi_env env_;
+ napi_finalize finalize_callback_;
+ void* finalize_data_;
+ void* finalize_hint_;
};
class TryCatch : public v8::TryCatch {
@@ -365,12 +339,22 @@ class TryCatch : public v8::TryCatch {
napi_env _env;
};
-// Wrapper around v8impl::Persistent that implements reference counting.
-class RefBase : protected Finalizer, RefTracker {
+// Ownership of a reference.
+enum class Ownership {
+ // The reference is owned by the runtime. No userland call is needed to
+ // destruct the reference.
+ kRuntime,
+ // The reference is owned by the userland. User code is responsible to delete
+ // the reference with appropriate node-api calls.
+ kUserland,
+};
+
+// Wrapper around Finalizer that implements reference counting.
+class RefBase : public Finalizer, public RefTracker {
protected:
RefBase(napi_env env,
uint32_t initial_refcount,
- bool delete_self,
+ Ownership ownership,
napi_finalize finalize_callback,
void* finalize_data,
void* finalize_hint);
@@ -378,30 +362,29 @@ class RefBase : protected Finalizer, RefTracker {
public:
static RefBase* New(napi_env env,
uint32_t initial_refcount,
- bool delete_self,
+ Ownership ownership,
napi_finalize finalize_callback,
void* finalize_data,
void* finalize_hint);
-
- static inline void Delete(RefBase* reference);
-
virtual ~RefBase();
+
void* Data();
uint32_t Ref();
uint32_t Unref();
uint32_t RefCount();
+ Ownership ownership() { return ownership_; }
+
protected:
- void Finalize(bool is_env_teardown = false) override;
+ void Finalize() override;
private:
- uint32_t _refcount;
- bool _delete_self;
+ uint32_t refcount_;
+ Ownership ownership_;
};
+// Wrapper around v8impl::Persistent.
class Reference : public RefBase {
- using SecondPassCallParameterRef = Reference*;
-
protected:
template <typename... Args>
Reference(napi_env env, v8::Local<v8::Value> value, Args&&... args);
@@ -410,7 +393,7 @@ class Reference : public RefBase {
static Reference* New(napi_env env,
v8::Local<v8::Value> value,
uint32_t initial_refcount,
- bool delete_self,
+ Ownership ownership,
napi_finalize finalize_callback = nullptr,
void* finalize_data = nullptr,
void* finalize_hint = nullptr);
@@ -421,22 +404,14 @@ class Reference : public RefBase {
v8::Local<v8::Value> Get();
protected:
- void Finalize(bool is_env_teardown = false) override;
+ void Finalize() override;
private:
- void ClearWeak();
- void SetWeak();
+ static void WeakCallback(const v8::WeakCallbackInfo<Reference>& data);
- static void FinalizeCallback(
- const v8::WeakCallbackInfo<SecondPassCallParameterRef>& data);
- static void SecondPassCallback(
- const v8::WeakCallbackInfo<SecondPassCallParameterRef>& data);
-
- v8impl::Persistent<v8::Value> _persistent;
- SecondPassCallParameterRef* _secondPassParameter;
- bool _secondPassScheduled;
+ void SetWeak();
- FRIEND_TEST(JsNativeApiV8Test, Reference);
+ v8impl::Persistent<v8::Value> persistent_;
};
} // end of namespace v8impl
diff --git a/src/node_api.cc b/src/node_api.cc
index 8f1e4fb0e4..c3f78c8b58 100644
--- a/src/node_api.cc
+++ b/src/node_api.cc
@@ -27,6 +27,7 @@ node_napi_env__::node_napi_env__(v8::Local<v8::Context> context,
void node_napi_env__::DeleteMe() {
destructing = true;
+ DrainFinalizerQueue();
napi_env__::DeleteMe();
}
@@ -47,26 +48,38 @@ void node_napi_env__::CallFinalizer(napi_finalize cb, void* data, void* hint) {
template <bool enforceUncaughtExceptionPolicy>
void node_napi_env__::CallFinalizer(napi_finalize cb, void* data, void* hint) {
- if (destructing) {
- // we can not defer finalizers when the environment is being destructed.
- v8::HandleScope handle_scope(isolate);
- v8::Context::Scope context_scope(context());
- CallbackIntoModule<enforceUncaughtExceptionPolicy>(
- [&](napi_env env) { cb(env, data, hint); });
- return;
+ v8::HandleScope handle_scope(isolate);
+ v8::Context::Scope context_scope(context());
+ CallbackIntoModule<enforceUncaughtExceptionPolicy>(
+ [&](napi_env env) { cb(env, data, hint); });
+}
+
+void node_napi_env__::EnqueueFinalizer(v8impl::RefTracker* finalizer) {
+ napi_env__::EnqueueFinalizer(finalizer);
+ // Schedule a second pass only when it has not been scheduled, and not
+ // destructing the env.
+ // When the env is being destructed, queued finalizers are drained in the
+ // loop of `node_napi_env__::DrainFinalizerQueue`.
+ if (!finalization_scheduled && !destructing) {
+ finalization_scheduled = true;
+ Ref();
+ node_env()->SetImmediate([this](node::Environment* node_env) {
+ finalization_scheduled = false;
+ Unref();
+ DrainFinalizerQueue();
+ });
+ }
+}
+
+void node_napi_env__::DrainFinalizerQueue() {
+ // As userland code can delete additional references in one finalizer,
+ // the list of pending finalizers may be mutated as we execute them, so
+ // we keep iterating it until it is empty.
+ while (!pending_finalizers.empty()) {
+ v8impl::RefTracker* ref_tracker = *pending_finalizers.begin();
+ pending_finalizers.erase(ref_tracker);
+ ref_tracker->Finalize();
}
- // we need to keep the env live until the finalizer has been run
- // EnvRefHolder provides an exception safe wrapper to Ref and then
- // Unref once the lambda is freed
- EnvRefHolder liveEnv(static_cast<napi_env>(this));
- node_env()->SetImmediate(
- [=, liveEnv = std::move(liveEnv)](node::Environment* node_env) {
- node_napi_env__* env = static_cast<node_napi_env__*>(liveEnv.env());
- v8::HandleScope handle_scope(env->isolate);
- v8::Context::Scope context_scope(env->context());
- env->CallbackIntoModule<enforceUncaughtExceptionPolicy>(
- [&](napi_env env) { cb(env, data, hint); });
- });
}
void node_napi_env__::trigger_fatal_exception(v8::Local<v8::Value> local_err) {
@@ -106,23 +119,40 @@ namespace {
class BufferFinalizer : private Finalizer {
public:
+ static BufferFinalizer* New(napi_env env,
+ napi_finalize finalize_callback = nullptr,
+ void* finalize_data = nullptr,
+ void* finalize_hint = nullptr) {
+ return new BufferFinalizer(
+ env, finalize_callback, finalize_data, finalize_hint);
+ }
// node::Buffer::FreeCallback
static void FinalizeBufferCallback(char* data, void* hint) {
std::unique_ptr<BufferFinalizer, Deleter> finalizer{
static_cast<BufferFinalizer*>(hint)};
- finalizer->_finalize_data = data;
+ finalizer->finalize_data_ = data;
- if (finalizer->_finalize_callback == nullptr) return;
- finalizer->_env->CallFinalizer(finalizer->_finalize_callback,
- finalizer->_finalize_data,
- finalizer->_finalize_hint);
+ // It is safe to call into JavaScript at this point.
+ if (finalizer->finalize_callback_ == nullptr) return;
+ finalizer->env_->CallFinalizer(finalizer->finalize_callback_,
+ finalizer->finalize_data_,
+ finalizer->finalize_hint_);
}
struct Deleter {
- void operator()(BufferFinalizer* finalizer) {
- Finalizer::Delete(finalizer);
- }
+ void operator()(BufferFinalizer* finalizer) { delete finalizer; }
};
+
+ private:
+ BufferFinalizer(napi_env env,
+ napi_finalize finalize_callback,
+ void* finalize_data,
+ void* finalize_hint)
+ : Finalizer(env, finalize_callback, finalize_data, finalize_hint) {
+ env_->Ref();
+ }
+
+ ~BufferFinalizer() { env_->Unref(); }
};
inline napi_env NewEnv(v8::Local<v8::Context> context,
@@ -371,10 +401,7 @@ class ThreadSafeFunction : public node::AsyncResource {
v8::HandleScope scope(env->isolate);
if (finalize_cb) {
CallbackScope cb_scope(this);
- // Do not use CallFinalizer since it will defer the invocation, which
- // would lead to accessing a deleted ThreadSafeFunction.
- env->CallbackIntoModule<false>(
- [&](napi_env env) { finalize_cb(env, finalize_data, context); });
+ env->CallFinalizer<false>(finalize_cb, finalize_data, context);
}
EmptyQueueAndDelete();
}
@@ -959,12 +986,8 @@ napi_status NAPI_CDECL napi_create_external_buffer(napi_env env,
v8::Isolate* isolate = env->isolate;
// The finalizer object will delete itself after invoking the callback.
- v8impl::Finalizer* finalizer =
- v8impl::Finalizer::New(env,
- finalize_cb,
- nullptr,
- finalize_hint,
- v8impl::Finalizer::kKeepEnvReference);
+ v8impl::BufferFinalizer* finalizer =
+ v8impl::BufferFinalizer::New(env, finalize_cb, nullptr, finalize_hint);
v8::MaybeLocal<v8::Object> maybe =
node::Buffer::New(isolate,
diff --git a/src/node_api_internals.h b/src/node_api_internals.h
index de5d9dc080..8b4db661e6 100644
--- a/src/node_api_internals.h
+++ b/src/node_api_internals.h
@@ -19,6 +19,9 @@ struct node_napi_env__ : public napi_env__ {
template <bool enforceUncaughtExceptionPolicy>
void CallFinalizer(napi_finalize cb, void* data, void* hint);
+ void EnqueueFinalizer(v8impl::RefTracker* finalizer) override;
+ void DrainFinalizerQueue();
+
void trigger_fatal_exception(v8::Local<v8::Value> local_err);
template <bool enforceUncaughtExceptionPolicy, typename T>
void CallbackIntoModule(T&& call);
@@ -32,6 +35,7 @@ struct node_napi_env__ : public napi_env__ {
std::string filename;
bool destructing = false;
+ bool finalization_scheduled = false;
};
using node_napi_env = node_napi_env__*;
diff --git a/test/cctest/test_js_native_api_v8.cc b/test/cctest/test_js_native_api_v8.cc
deleted file mode 100644
index 06de770089..0000000000
--- a/test/cctest/test_js_native_api_v8.cc
+++ /dev/null
@@ -1,102 +0,0 @@
-#include <stdio.h>
-#include <cstdio>
-#include <string>
-#include "env-inl.h"
-#include "gtest/gtest.h"
-#include "js_native_api_v8.h"
-#include "node_api_internals.h"
-#include "node_binding.h"
-#include "node_test_fixture.h"
-
-namespace v8impl {
-
-using v8::Local;
-using v8::Object;
-
-static napi_env addon_env;
-static uint32_t finalizer_call_count = 0;
-
-class JsNativeApiV8Test : public EnvironmentTestFixture {
- private:
- void SetUp() override {
- EnvironmentTestFixture::SetUp();
- finalizer_call_count = 0;
- }
-
- void TearDown() override { NodeTestFixture::TearDown(); }
-};
-
-TEST_F(JsNativeApiV8Test, Reference) {
- const v8::HandleScope handle_scope(isolate_);
- Argv argv;
-
- napi_ref ref;
- void* embedder_fields[v8::kEmbedderFieldsInWeakCallback] = {nullptr, nullptr};
- v8::WeakCallbackInfo<Reference::SecondPassCallParameterRef>::Callback
- callback;
- Reference::SecondPassCallParameterRef* parameter = nullptr;
-
- {
- Env test_env{handle_scope, argv};
-
- node::Environment* env = *test_env;
- node::LoadEnvironment(env, "");
-
- napi_addon_register_func init = [](napi_env env, napi_value exports) {
- addon_env = env;
- return exports;
- };
- Local<Object> module_obj = Object::New(isolate_);
- Local<Object> exports_obj = Object::New(isolate_);
- napi_module_register_by_symbol(
- exports_obj, module_obj, env->context(), init);
- ASSERT_NE(addon_env, nullptr);
- node_napi_env internal_env = reinterpret_cast<node_napi_env>(addon_env);
- EXPECT_EQ(internal_env->node_env(), env);
-
- // Create a new scope to manage the handles.
- {
- const v8::HandleScope handle_scope(isolate_);
- napi_value value;
- napi_create_object(addon_env, &value);
- // Create a weak reference;
- napi_add_finalizer(
- addon_env,
- value,
- nullptr,
- [](napi_env env, void* finalize_data, void* finalize_hint) {
- finalizer_call_count++;
- },
- nullptr,
- &ref);
- parameter = reinterpret_cast<Reference*>(ref)->_secondPassParameter;
- }
-
- // We can hardly trigger a non-forced Garbage Collection in a stable way.
- // Here we just invoke the weak callbacks directly.
- // The persistent handles should be reset in the weak callback in respect
- // to the API contract of v8 weak callbacks.
- v8::WeakCallbackInfo<Reference::SecondPassCallParameterRef> data(
- reinterpret_cast<v8::Isolate*>(isolate_),
- parameter,
- embedder_fields,
- &callback);
- Reference::FinalizeCallback(data);
- EXPECT_EQ(callback, &Reference::SecondPassCallback);
- }
- // Env goes out of scope, the environment has been teardown. All node-api ref
- // trackers should have been destroyed.
-
- // Now we call the second pass callback to verify the method do not abort with
- // memory violations.
- v8::WeakCallbackInfo<Reference::SecondPassCallParameterRef> data(
- reinterpret_cast<v8::Isolate*>(isolate_),
- parameter,
- embedder_fields,
- nullptr);
- Reference::SecondPassCallback(data);
-
- // After Environment Teardown
- EXPECT_EQ(finalizer_call_count, uint32_t(1));
-}
-} // namespace v8impl
diff --git a/test/js-native-api/6_object_wrap/6_object_wrap.cc b/test/js-native-api/6_object_wrap/6_object_wrap.cc
index 7f8fdd3416..7ebe711a6d 100644
--- a/test/js-native-api/6_object_wrap/6_object_wrap.cc
+++ b/test/js-native-api/6_object_wrap/6_object_wrap.cc
@@ -1,5 +1,6 @@
-#include "myobject.h"
#include "../common.h"
+#include "assert.h"
+#include "myobject.h"
napi_ref MyObject::constructor;
@@ -151,9 +152,69 @@ napi_value MyObject::Multiply(napi_env env, napi_callback_info info) {
return instance;
}
+// This finalizer should never be invoked.
+void ObjectWrapDanglingReferenceFinalizer(napi_env env,
+ void* finalize_data,
+ void* finalize_hint) {
+ assert(0 && "unreachable");
+}
+
+napi_ref dangling_ref;
+napi_value ObjectWrapDanglingReference(napi_env env, napi_callback_info info) {
+ size_t argc = 1;
+ napi_value args[1];
+ NODE_API_CALL(env,
+ napi_get_cb_info(env, info, &argc, args, nullptr, nullptr));
+
+ // Create a napi_wrap and remove it immediately, whilst leaving the out-param
+ // ref dangling (not deleted).
+ NODE_API_CALL(env,
+ napi_wrap(env,
+ args[0],
+ nullptr,
+ ObjectWrapDanglingReferenceFinalizer,
+ nullptr,
+ &dangling_ref));
+ NODE_API_CALL(env, napi_remove_wrap(env, args[0], nullptr));
+
+ return args[0];
+}
+
+napi_value ObjectWrapDanglingReferenceTest(napi_env env,
+ napi_callback_info info) {
+ napi_value out;
+ napi_value ret;
+ NODE_API_CALL(env, napi_get_reference_value(env, dangling_ref, &out));
+
+ if (out == nullptr) {
+ // If the napi_ref has been invalidated, delete it.
+ NODE_API_CALL(env, napi_delete_reference(env, dangling_ref));
+ NODE_API_CALL(env, napi_get_boolean(env, true, &ret));
+ } else {
+ // The dangling napi_ref is still valid.
+ NODE_API_CALL(env, napi_get_boolean(env, false, &ret));
+ }
+ return ret;
+}
+
EXTERN_C_START
napi_value Init(napi_env env, napi_value exports) {
MyObject::Init(env, exports);
+
+ napi_property_descriptor descriptors[] = {
+ DECLARE_NODE_API_PROPERTY("objectWrapDanglingReference",
+ ObjectWrapDanglingReference),
+ DECLARE_NODE_API_PROPERTY("objectWrapDanglingReferenceTest",
+ ObjectWrapDanglingReferenceTest),
+ };
+
+ NODE_API_CALL(
+ env,
+ napi_define_properties(env,
+ exports,
+ sizeof(descriptors) / sizeof(*descriptors),
+ descriptors));
+
return exports;
}
EXTERN_C_END
diff --git a/test/js-native-api/6_object_wrap/test-object-wrap-ref.js b/test/js-native-api/6_object_wrap/test-object-wrap-ref.js
new file mode 100644
index 0000000000..81832c195c
--- /dev/null
+++ b/test/js-native-api/6_object_wrap/test-object-wrap-ref.js
@@ -0,0 +1,13 @@
+// Flags: --expose-gc
+
+'use strict';
+const common = require('../../common');
+const addon = require(`./build/${common.buildType}/6_object_wrap`);
+
+(function scope() {
+ addon.objectWrapDanglingReference({});
+})();
+
+common.gcUntil('object-wrap-ref', () => {
+ return addon.objectWrapDanglingReferenceTest();
+});
View Lorry Controller Status