Exceptions in constructors causes v8 GC to fail with access violations

[blagoev]

Hey folks,
If an exception is thrown from inside a constructor it seems it causes v8 GC to fail with an access violation in this case below. I am starting to think that this is caused by ObjectWrap<T>::FinalizeCallback being called when it shouldn't be.

Here is the example code if you want to try it out quickly blagoev/node-addon-examples@351752f

The highlights are these.

There is a static factory function on the exposed object

Napi::Value create(const Napi::CallbackInfo& info) {
	Napi::EscapableHandleScope scope(info.Env());
	Napi::Value val = MyObject::constructor.New({}); //this will throw an exception

	return scope.Escape(info.Env().Undefined());
}

then the ctor throws some error

MyObject::MyObject(const Napi::CallbackInfo& info)
	: Napi::ObjectWrap<MyObject>(info) {
	Napi::Env env = info.Env();
	Napi::HandleScope scope(env);
	throw Napi::Error::New(env, "TEST");
......

and we have this private field in the class

struct Data {
};

class MyObject : public Napi::ObjectWrap<MyObject> {
private:
	std::shared_ptr<Data> m_data;
....

and this is the JS code that tries to call that

var addon = require('bindings')('addon');
try {
    var obj = addon.MyObject.create();
    }
    catch (e) {
        console.error("Incorrect expected exception message:" + e.message);
    }
  
    global.gc()
    global.gc()
    global.gc()

then there is an exception here

>	addon.node!std::_Ref_count_base::_Decref() Line 845	C++
 	addon.node!std::_Ptr_base<Data>::_Decref() Line 1122	C++
 	addon.node!std::shared_ptr<Data>::~shared_ptr<Data>() Line 1403	C++
 	addon.node!MyObject::~MyObject()	C++
 	addon.node!MyObject::`scalar deleting destructor'(unsigned int)	C++
 	addon.node!Napi::ObjectWrap<MyObject>::FinalizeCallback(napi_env__ * __formal, void * data, void * __formal) Line 3433	C++
 	node.exe!`anonymous namespace'::v8impl::Reference::SecondPassCallback(const v8::WeakCallbackInfo<`anonymous namespace'::v8impl::Reference> & data) Line 496	C++
 	[Inline Frame] node.exe!v8::internal::GlobalHandles::PendingPhantomCallback::Invoke(v8::internal::Isolate *) Line 905	C++
 	node.exe!v8::internal::GlobalHandles::InvokeSecondPassPhantomCallbacks(std::vector<v8::internal::GlobalHandles::PendingPhantomCallback,std::allocator<v8::internal::GlobalHandles::PendingPhantomCallback>> * callbacks, v8::internal::Isolate * isolate) Line 772	C++
 	node.exe!v8::internal::GlobalHandles::DispatchPendingPhantomCallbacks(bool synchronous_second_pass) Line 881	C++
 	node.exe!v8::internal::GlobalHandles::PostGarbageCollectionProcessing(v8::internal::GarbageCollector collector, const v8::GCCallbackFlags gc_callback_flags) Line 930	C++
 	node.exe!v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector collector, const v8::GCCallbackFlags gc_callback_flags) Line 1775	C++
 	node.exe!v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace space, v8::internal::GarbageCollectionReason gc_reason, const v8::GCCallbackFlags gc_callback_flags) Line 1392	C++
 	[Inline Frame] node.exe!v8::internal::Heap::CollectAllGarbage(int) Line 1156	C++
 	node.exe!v8::Isolate::RequestGarbageCollectionForTesting(v8::Isolate::GarbageCollectionType type) Line 8273	C++
 	node.exe!v8::internal::GCExtension::GC(const v8::FunctionCallbackInfo<v8::Value> & args) Line 26	C++
 	node.exe!v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo * handler) Line 95	C++
 	node.exe!v8::internal::`anonymous namespace'::HandleApiCallHelper<0>(v8::internal::Isolate * isolate, v8::internal::Handle<v8::internal::HeapObject> new_target, v8::internal::Handle<v8::internal::HeapObject> fun_data, v8::internal::Handle<v8::internal::FunctionTemplateInfo> receiver, v8::internal::Handle<v8::internal::Object> args, v8::internal::BuiltinArguments) Line 111	C++
 	node.exe!v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments args, v8::internal::Isolate * isolate) Line 139	C++
 	node.exe!v8::internal::Builtin_HandleApiCall(int args_length, v8::internal::Object * * args_object, v8::internal::Isolate * isolate) Line 127	C++

Note: you need to expose v8 GC for that to work with

node --expose_gc addon.js

It seems to me this is cause the ObjectWrap<T>::FinalizeCallback is called but the native object did not get constructed correctly (ie the native object's ctor did not complete). Or is it that my assumption is wrong.

cheers

[blagoev]

To elaborate some more: I think the issue is that registering the FinalizeCallback happens before the child class MyObject constructor code is executed and that napi_wrap call captures the pointer to this, which is not completely initialized. Then later GC will cal the FinalizeCallback which will try to delete instance.
This will fail even without the std::shared_ptr member (with different call stack)

cheers

[gabrielschulhof]

IINM this is a duplicate of #599.