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: #300 Reviewed-By: Michael Dawson <[email protected]>
nodejs · Jul 14, 2018 · 2ff776f · 2ff776f
1 parent 9816197
commit 2ff776f
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 8 deletions.
diff --git 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<v8::Value> 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<v8::Value> handle;  // Die with this JavaScript object
+  node::Persistent<v8::Value> handle;  // Die with this JavaScript object
+
  private:
   static void WeakCallback(v8::WeakCallbackInfo<CallbackBundle> const& info) {
     // Use the "WeakCallback mechanism" to delete the C++ `bundle` object.

diff --git 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 <typename T>
+struct ResetInDestructorPersistentTraits {
+  static const bool kResetInDestructor = true;
+  template <typename S, typename M>
+  // Disallow copy semantics by leaving this unimplemented.
+  inline static void Copy(
+      const v8::Persistent<S, M>&,
+      v8::Persistent<T, ResetInDestructorPersistentTraits<T>>*);
+};
+
+// 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 <typename T>
+using Persistent = v8::Persistent<T, ResetInDestructorPersistentTraits<T>>;
+
 #if NODE_MAJOR_VERSION < 8 || NODE_MAJOR_VERSION == 8 && NODE_MINOR_VERSION < 2
 typedef int async_id;