From ba7ad37d4462c5f3b61a8a7750df2906b7914609 Mon Sep 17 00:00:00 2001
From: David Halls <dahalls@gmail.com>
Date: Sat, 16 May 2020 08:36:06 +0100
Subject: [PATCH] src: fix ObjectWrap inheritance

- fix wrap/unwrap of objects inheriting from non-ObjectWrap

PR-URL: https://github.com/nodejs/node-addon-api/pull/732
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
---
 napi-inl.h                              |  7 +++---
 test/binding.cc                         |  2 ++
 test/binding.gyp                        |  1 +
 test/index.js                           |  1 +
 test/objectwrap_multiple_inheritance.cc | 30 +++++++++++++++++++++++++
 test/objectwrap_multiple_inheritance.js | 15 +++++++++++++
 6 files changed, 53 insertions(+), 3 deletions(-)
 create mode 100644 test/objectwrap_multiple_inheritance.cc
 create mode 100644 test/objectwrap_multiple_inheritance.js

diff --git a/napi-inl.h b/napi-inl.h
index 7e4d158db..90a295005 100644
--- a/napi-inl.h
+++ b/napi-inl.h
@@ -3146,10 +3146,11 @@ inline ObjectWrap<T>::ObjectWrap(const Napi::CallbackInfo& callbackInfo) {
   napi_value wrapper = callbackInfo.This();
   napi_status status;
   napi_ref ref;
-  status = napi_wrap(env, wrapper, this, FinalizeCallback, nullptr, &ref);
+  T* instance = static_cast<T*>(this);
+  status = napi_wrap(env, wrapper, instance, FinalizeCallback, nullptr, &ref);
   NAPI_THROW_IF_FAILED_VOID(env, status);
 
-  Reference<Object>* instanceRef = this;
+  Reference<Object>* instanceRef = instance;
   *instanceRef = Reference<Object>(env, ref);
 }
 
@@ -3872,7 +3873,7 @@ inline napi_value ObjectWrap<T>::InstanceSetterCallbackWrapper(
 
 template <typename T>
 inline void ObjectWrap<T>::FinalizeCallback(napi_env env, void* data, void* /*hint*/) {
-  ObjectWrap<T>* instance = static_cast<ObjectWrap<T>*>(data);
+  T* instance = static_cast<T*>(data);
   instance->Finalize(Napi::Env(env));
   delete instance;
 }
diff --git a/test/binding.cc b/test/binding.cc
index ea1094638..829b45e6b 100644
--- a/test/binding.cc
+++ b/test/binding.cc
@@ -53,6 +53,7 @@ Object InitTypedArray(Env env);
 Object InitObjectWrap(Env env);
 Object InitObjectWrapConstructorException(Env env);
 Object InitObjectWrapRemoveWrap(Env env);
+Object InitObjectWrapMultipleInheritance(Env env);
 Object InitObjectReference(Env env);
 Object InitVersionManagement(Env env);
 Object InitThunkingManual(Env env);
@@ -111,6 +112,7 @@ Object Init(Env env, Object exports) {
   exports.Set("objectwrapConstructorException",
       InitObjectWrapConstructorException(env));
   exports.Set("objectwrap_removewrap", InitObjectWrapRemoveWrap(env));
+  exports.Set("objectwrap_multiple_inheritance", InitObjectWrapMultipleInheritance(env));
   exports.Set("objectreference", InitObjectReference(env));
   exports.Set("version_management", InitVersionManagement(env));
   exports.Set("thunking_manual", InitThunkingManual(env));
diff --git a/test/binding.gyp b/test/binding.gyp
index 8dafe13fc..2c6f791b1 100644
--- a/test/binding.gyp
+++ b/test/binding.gyp
@@ -45,6 +45,7 @@
         'objectwrap.cc',
         'objectwrap_constructor_exception.cc',
         'objectwrap-removewrap.cc',
+        'objectwrap_multiple_inheritance.cc',
         'objectreference.cc',
         'version_management.cc',
         'thunking_manual.cc',
diff --git a/test/index.js b/test/index.js
index 78c1c9779..09b40c25d 100644
--- a/test/index.js
+++ b/test/index.js
@@ -53,6 +53,7 @@ let testModules = [
   'objectwrap',
   'objectwrap_constructor_exception',
   'objectwrap-removewrap',
+  'objectwrap_multiple_inheritance',
   'objectreference',
   'version_management'
 ];
diff --git a/test/objectwrap_multiple_inheritance.cc b/test/objectwrap_multiple_inheritance.cc
new file mode 100644
index 000000000..67913eb4e
--- /dev/null
+++ b/test/objectwrap_multiple_inheritance.cc
@@ -0,0 +1,30 @@
+#include <napi.h>
+
+class TestMIBase {
+public:
+  TestMIBase() : test(0) {}
+  virtual void dummy() {}
+  uint32_t test;
+};
+
+class TestMI : public TestMIBase, public Napi::ObjectWrap<TestMI> {
+public:
+  TestMI(const Napi::CallbackInfo& info) :
+    Napi::ObjectWrap<TestMI>(info) {}
+
+  Napi::Value GetTest(const Napi::CallbackInfo& info) {
+    return Napi::Number::New(info.Env(), test);
+  }
+
+  static void Initialize(Napi::Env env, Napi::Object exports) {
+    exports.Set("TestMI", DefineClass(env, "TestMI", {
+      InstanceAccessor<&TestMI::GetTest>("test")
+    }));
+  }
+};
+
+Napi::Object InitObjectWrapMultipleInheritance(Napi::Env env) {
+  Napi::Object exports = Napi::Object::New(env);
+  TestMI::Initialize(env, exports);
+  return exports;
+}
diff --git a/test/objectwrap_multiple_inheritance.js b/test/objectwrap_multiple_inheritance.js
new file mode 100644
index 000000000..87c669eb3
--- /dev/null
+++ b/test/objectwrap_multiple_inheritance.js
@@ -0,0 +1,15 @@
+'use strict';
+
+const buildType = process.config.target_defaults.default_configuration;
+const assert = require('assert');
+
+const test = bindingName => {
+  const binding = require(bindingName);
+  const TestMI = binding.objectwrap_multiple_inheritance.TestMI;
+  const testmi = new TestMI();
+
+  assert.strictEqual(testmi.test, 0);
+}
+
+test(`./build/${buildType}/binding.node`);
+test(`./build/${buildType}/binding_noexcept.node`);