diff --git a/napi-inl.h b/napi-inl.h index bef889f9a..118f1d479 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -2711,6 +2711,27 @@ inline Object FunctionReference::New(const std::vector& args) const return scope.Escape(Value().New(args)).As(); } +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::ObjectWrap(const Napi::CallbackInfo& callbackInfo) { napi_status status; napi_ref ref; T* instance = static_cast(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(callbackInfo)); + delete finalizerHint; + } NAPI_THROW_IF_FAILED_VOID(env, status); Reference* instanceRef = instance; @@ -3697,10 +3723,31 @@ inline napi_value ObjectWrap::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::InstanceSetterCallbackWrapper( } template -inline void ObjectWrap::FinalizeCallback(napi_env env, void* data, void* /*hint*/) { +inline void ObjectWrap::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(data); instance->Finalize(Napi::Env(env)); delete instance; diff --git a/napi.h b/napi.h index 62198b1b3..001a92490 100644 --- a/napi.h +++ b/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 { diff --git a/test/objectwrap.cc b/test/objectwrap.cc index 0d61171ca..f0d363370 100644 --- a/test/objectwrap.cc +++ b/test/objectwrap.cc @@ -172,11 +172,35 @@ class Test : public Napi::ObjectWrap { std::string Test::s_staticMethodText; + +class TestConstructorExceptions : public Napi::ObjectWrap { +public: + TestConstructorExceptions(const Napi::CallbackInfo& info) : + Napi::ObjectWrap(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; } diff --git a/test/objectwrap.js b/test/objectwrap.js index de16a6067..0ce63e7f5 100644 --- a/test/objectwrap.js +++ b/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`));