fixes v8 GC access violation for zombie objects

napi-inl.h

@@ -2711,6 +2711,27 @@ inline Object FunctionReference::New(const std::vector<napi_value>& args) const
   return scope.Escape(Value().New(args)).As<Object>();
 }
 
+class FinalizerHint {
+  private:
+    FinalizerHint() {}
+  public:
+    bool shouldFinalize;
+
+    static void Init(CallbackInfo& info, bool shouldFinalize = true) {
+      info.finalizerHint = new FinalizerHint(); 
+      info.finalizerHint->shouldFinalize = shouldFinalize;
+    }
+
+    static void Clear(CallbackInfo& info) {
+      info.finalizerHint = nullptr;
+    }
+
+    static FinalizerHint* Get(const CallbackInfo& info) {
+      return info.finalizerHint;
+    }
+};
+
+
 ////////////////////////////////////////////////////////////////////////////////
 // CallbackInfo class
 ////////////////////////////////////////////////////////////////////////////////
@@ -3120,7 +3141,12 @@ inline ObjectWrap<T>::ObjectWrap(const Napi::CallbackInfo& callbackInfo) {
   napi_status status;
   napi_ref ref;
   T* instance = static_cast<T*>(this);
-  status = napi_wrap(env, wrapper, instance, FinalizeCallback, nullptr, &ref);
+  FinalizerHint* finalizerHint = FinalizerHint::Get(callbackInfo);
+  status = napi_wrap(env, wrapper, instance, FinalizeCallback, (void*)finalizerHint, &ref);
+  if (status != napi_ok && finalizerHint != nullptr) {
+      FinalizerHint::Clear(const_cast<Napi::CallbackInfo&>(callbackInfo));
+      delete finalizerHint;
+  }
   NAPI_THROW_IF_FAILED_VOID(env, status);
 
   Reference<Object>* instanceRef = instance;
@@ -3697,10 +3723,31 @@ inline napi_value ObjectWrap<T>::ConstructorCallbackWrapper(
   }
 
   T* instance;
-  napi_value wrapper = details::WrapCallback([&] {
+  napi_value wrapper = details::WrapCallback([&] () -> napi_value {
     CallbackInfo callbackInfo(env, info);
+    FinalizerHint::Init(callbackInfo);
+#ifdef NAPI_CPP_EXCEPTIONS
+    try {
+      instance = new T(callbackInfo);
+      return callbackInfo.This();
+    }
+    catch (...) {
+      FinalizerHint* finalizerHint = FinalizerHint::Get(callbackInfo);
+      if (finalizerHint != nullptr) {
+         finalizerHint->shouldFinalize = false;
+      }
+      throw;
+    }
+#else
     instance = new T(callbackInfo);
+    if (callbackInfo.Env().IsExceptionPending()) {
+      FinalizerHint* finalizerHint = FinalizerHint::Get(callbackInfo);
+      if (finalizerHint != nullptr) {
+         finalizerHint->shouldFinalize = false;
+      }
+    }
     return callbackInfo.This();
+#endif
   });
 
   return wrapper;
@@ -3823,7 +3870,16 @@ inline napi_value ObjectWrap<T>::InstanceSetterCallbackWrapper(
 }
 
 template <typename T>
-inline void ObjectWrap<T>::FinalizeCallback(napi_env env, void* data, void* /*hint*/) {
+inline void ObjectWrap<T>::FinalizeCallback(napi_env env, void* data, void* hint) {
+ if (hint != nullptr) {
+    FinalizerHint* finalizerHint = (FinalizerHint*)hint;
+    bool shouldFinalize = finalizerHint->shouldFinalize;
+    delete finalizerHint;
+    if (!shouldFinalize) {
+      return;
+    }
+  }
+
   T* instance = reinterpret_cast<T*>(data);
   instance->Finalize(Napi::Env(env));
   delete instance;

napi.h

@@ -1397,6 +1397,7 @@ namespace Napi {
   };
 
   class CallbackInfo {
+    friend class FinalizerHint;
   public:
     CallbackInfo(napi_env env, napi_callback_info info);
     ~CallbackInfo();
@@ -1424,6 +1425,7 @@ namespace Napi {
     napi_value _staticArgs[6];
     napi_value* _dynamicArgs;
     void* _data;
+    FinalizerHint* finalizerHint;
   };
 
   class PropertyDescriptor {

test/objectwrap.cc

@@ -172,11 +172,35 @@ class Test : public Napi::ObjectWrap<Test> {
 
 std::string Test::s_staticMethodText;
 
+
+class TestConstructorExceptions : public Napi::ObjectWrap<TestConstructorExceptions> {
+public:
+  TestConstructorExceptions(const Napi::CallbackInfo& info) : 
+    Napi::ObjectWrap<TestConstructorExceptions>(info) {
+      #ifdef NAPI_CPP_EXCEPTIONS
+        throw Napi::Error::New(info.Env(), "Constructor exceptions should not cause v8 GC to fail");
+      #else
+        Napi::Error::New(info.Env(), "Constructor exceptions should not cause v8 GC to fail").ThrowAsJavaScriptException();
+        return;
+      #endif
+    }
+
+    static void Initialize(Napi::Env env, Napi::Object exports) {
+      exports.Set("TestConstructorExceptions", DefineClass(env, "TestConstructorExceptions", {}));
+    }
+};
+
+
+
 Napi::Object InitObjectWrap(Napi::Env env) {
   testStaticContextRef = Napi::Persistent(Napi::Object::New(env));
   testStaticContextRef.SuppressDestruct();
 
   Napi::Object exports = Napi::Object::New(env);
   Test::Initialize(env, exports);
+
+#ifdef NAPI_CPP_EXCEPTIONS
+  TestConstructorExceptions::Initialize(env, exports);
+#endif
   return exports;
 }

test/objectwrap.js

@@ -256,9 +256,19 @@ const test = (binding) => {
     testFinalize(clazz);
   };
 
+  const testConstructorExceptions = () => {
+    const TestConstructorExceptions = binding.objectwrap.TestConstructorExceptions;
+    console.log("Runnig test testConstructorExceptions");
+    assert.throws(() => { new TestConstructorExceptions(); });
+    global.gc();
+    console.log("Test testConstructorExceptions complete");
+  }
+
   // `Test` is needed for accessing exposed symbols
   testObj(new Test(), Test);
   testClass(Test);
+
+  testConstructorExceptions();
 }
 
 test(require(`./build/${buildType}/binding.node`));