Fix crash due to ObjectWrap copying CallbackInfo

PR-URL: #125
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>

Author: davedoesdev

napi-inl.h

@@ -2288,7 +2288,7 @@ inline PropertyDescriptor::operator const napi_property_descriptor&() const {
 ////////////////////////////////////////////////////////////////////////////////
 
 template <typename T>
-inline ObjectWrap<T>::ObjectWrap(Napi::CallbackInfo callbackInfo) {
+inline ObjectWrap<T>::ObjectWrap(const Napi::CallbackInfo& callbackInfo) {
   napi_env env = callbackInfo.Env();
   napi_value wrapper = callbackInfo.This();
   napi_status status;

napi.h

@@ -1125,6 +1125,10 @@ namespace Napi {
     CallbackInfo(napi_env env, napi_callback_info info);
     ~CallbackInfo();
 
+    // Disallow copying to prevent multiple free of _dynamicArgs
+    CallbackInfo(CallbackInfo const &) = delete;
+    void operator=(CallbackInfo const &) = delete;
+
     Napi::Env Env() const;
     Value NewTarget() const;
     bool IsConstructCall() const;
@@ -1279,7 +1283,7 @@ namespace Napi {
   template <typename T>
   class ObjectWrap : public Reference<Object> {
   public:
-    ObjectWrap(CallbackInfo callbackInfo);
+    ObjectWrap(const CallbackInfo& callbackInfo);
 
     static T* Unwrap(Object wrapper);
 

test/binding.cc

@@ -11,6 +11,7 @@ Object InitFunction(Env env);
 Object InitName(Env env);
 Object InitObject(Env env);
 Object InitTypedArray(Env env);
+Object InitObjectWrap(Env env);
 
 void Init(Env env, Object exports, Object module) {
   exports.Set("arraybuffer", InitArrayBuffer(env));
@@ -22,6 +23,7 @@ void Init(Env env, Object exports, Object module) {
   exports.Set("name", InitName(env));
   exports.Set("object", InitObject(env));
   exports.Set("typedarray", InitTypedArray(env));
+  exports.Set("objectwrap", InitObjectWrap(env));
 }
 
 NODE_API_MODULE(addon, Init)

test/binding.gyp

@@ -11,6 +11,7 @@
         'name.cc',
         'object.cc',
         'typedarray.cc',
+        'objectwrap.cc',
       ],
       'include_dirs': ["<!@(node -p \"require('../').include\")"],
       'dependencies': ["<!(node -p \"require('../').gyp\")"],

test/index.js

@@ -10,6 +10,7 @@ let testModules = [
   'name',
   'object',
   'typedarray',
+  'objectwrap'
 ];
 
 if (typeof global.gc === 'function') {

test/objectwrap.cc

@@ -0,0 +1,32 @@
+#include <napi.h>
+
+class Test : public Napi::ObjectWrap<Test> {
+public:
+  Test(const Napi::CallbackInfo& info) :
+    Napi::ObjectWrap<Test>(info) {
+  }
+
+  void Set(const Napi::CallbackInfo& info) {
+    value = info[0].As<Napi::Number>();
+  }
+
+  Napi::Value Get(const Napi::CallbackInfo& info) {
+    return Napi::Number::New(info.Env(), value);
+  }
+
+  static void Initialize(Napi::Env env, Napi::Object exports) {
+    exports.Set("Test", DefineClass(env, "Test", {
+      InstanceMethod("test_set", &Test::Set),
+      InstanceMethod("test_get", &Test::Get)
+    }));
+  }
+
+private:
+  uint32_t value;
+};
+
+Napi::Object InitObjectWrap(Napi::Env env) {
+  Napi::Object exports = Napi::Object::New(env);
+  Test::Initialize(env, exports);
+  return exports;
+}

test/objectwrap.js

@@ -0,0 +1,24 @@
+'use strict';
+const buildType = process.config.target_defaults.default_configuration;
+const assert = require('assert');
+
+test(require(`./build/${buildType}/binding.node`));
+test(require(`./build/${buildType}/binding_noexcept.node`));
+
+function test(binding) {
+  var Test = binding.objectwrap.Test;
+
+  function testSetGet(obj) {
+    obj.test_set(90);
+    assert.strictEqual(obj.test_get(), 90);
+  }
+
+  function testObj(obj) {
+    testSetGet(obj);
+  }
+
+  testObj(new Test());
+  testObj(new Test(1));
+  testObj(new Test(1, 2, 3, 4, 5, 6));
+  testObj(new Test(1, 2, 3, 4, 5, 6, 7));
+}