From 2ff776ffe36cb694506678edc68ebf17f8eacd89 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Thu, 12 Jul 2018 09:15:48 -0400 Subject: [PATCH] backport node::Persistent This reduces the delta in src/node_api.cc for the definition of the `CallbackBundle` structure back to zero wrt. upstream by copying the `node::Persistent` class template from upstream's src/node_persistent.h. PR-URL: https://github.com/nodejs/node-addon-api/pull/300 Reviewed-By: Michael Dawson --- src/node_api.cc | 11 +++-------- src/node_internals.h | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/node_api.cc b/src/node_api.cc index e717a3835..dd16016f3 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -498,13 +498,7 @@ class TryCatch : public v8::TryCatch { // calling through N-API. // Ref: benchmark/misc/function_call // Discussion (incl. perf. data): https://github.com/nodejs/node/pull/21072 -class CallbackBundle { - public: - - ~CallbackBundle() { - handle.ClearWeak(); - handle.Reset(); - } +struct CallbackBundle { // Bind the lifecycle of `this` C++ object to a JavaScript object. // We never delete a CallbackBundle C++ object directly. void BindLifecycleTo(v8::Isolate* isolate, v8::Local target) { @@ -516,7 +510,8 @@ class CallbackBundle { void* cb_data; // The user provided callback data napi_callback function_or_getter; napi_callback setter; - v8::Persistent handle; // Die with this JavaScript object + node::Persistent handle; // Die with this JavaScript object + private: static void WeakCallback(v8::WeakCallbackInfo const& info) { // Use the "WeakCallback mechanism" to delete the C++ `bundle` object. diff --git a/src/node_internals.h b/src/node_internals.h index 7444fbdd9..bacccffb7 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -70,6 +70,23 @@ class CallbackScope { namespace node { +// Copied from Node.js' src/node_persistent.h +template +struct ResetInDestructorPersistentTraits { + static const bool kResetInDestructor = true; + template + // Disallow copy semantics by leaving this unimplemented. + inline static void Copy( + const v8::Persistent&, + v8::Persistent>*); +}; + +// v8::Persistent does not reset the object slot in its destructor. That is +// acknowledged as a flaw in the V8 API and expected to change in the future +// but for now node::Persistent is the easier and safer alternative. +template +using Persistent = v8::Persistent>; + #if NODE_MAJOR_VERSION < 8 || NODE_MAJOR_VERSION == 8 && NODE_MINOR_VERSION < 2 typedef int async_id;