Repository navigation
Fatal error with ObjectWrap since 1.5.0 #372
Description
Activity
@BotellaA which precise versions of Node.js have you tried this with? We've had crashes caused by Node.js itself, because it was accessing a stale pointer as part of the execution of
OnOK.I tried 8.12, 10.11 and 10.12.
Changelog is here: https://github.com/nodejs/node-addon-api/blob/master/CHANGELOG.md
If we have them properly labelled then these should be the only candiates:
[917bd60] - src: remove TODOs by fixing memory leaks (Gabriel Schulhof) #343
[dfcb939] - src: implement AsyncContext class (Jinho Bang) #252
[211ed38] - src: make 'nothing' target a static library (Gabriel Schulhof) #348
[97c4ab5] - src: add Call and MakeCallback that accept cargs (NickNaso) #344
[b6e2d92] - src: enable DataView feature by default (Jinho) #331
[0a00e7c] - src: implement missing descriptor defs for symbols (Philipp Renoth) #280
[38e01b7] - src: first pass on adding version management apis (NickNaso) #325
[79ee838] - src: fix compile failure in test (Michael Dawson) #345
[4d92a60] - src: Add ObjectReference test case (Anisha Rohra) #212Guessing that we can eliminate ones which add new functionality, which I think leaves this one unless we have some problems with the instance of a method with the wrong parameters being selected because there are additional options is a possibility(for example [97c4ab5] - src: add Call and MakeCallback that accept cargs (NickNaso) #344)
[917bd60] - src: remove TODOs by fixing memory leaks (Gabriel Schulhof) #343
@gabrielschulhof since you are already engaged maybe you can help @BotellaA revert that change and see if it is related?
@BotellaA can you rebuild with
--debugso I can see line numbers on the node-addon-api part of the stack? I suspect that a function-valued property gets added as an instance method. This is not allowed.Also, can you point me to the source code, if it's open?
Here is the debug stack
#1 0x00000000016a8a94 in V8_Fatal(char const*, int, char const*, ...) () #2 0x0000000000b04353 in v8::Template::Set(v8::Local<v8::Name>, v8::Local<v8::Data>, v8::PropertyAttribute) () #3 0x00000000008eac00 in napi_define_class () #4 0x00007fffdfde0728 in Napi::ObjectWrap<genepi::ClassWrapper<Test> >::DefineClass (env=..., utf8name=0x7fffdffff978 <genepi::BindClass<Test>::getInstance()::instance+24> "Test", props_count=4, descriptors=0x251d1f0, data=0x0) at /home/camaud/programming/genepi/node_modules/node-addon-api/napi-inl.h:2882 #5 0x00007fffdfddf2c2 in Napi::ObjectWrap<genepi::ClassWrapper<Test> >::DefineClass (env=..., utf8name=0x7fffdffff978 <genepi::BindClass<Test>::getInstance()::instance+24> "Test", properties=std::vector of length 4, capacity 4 = {...}, data=0x0) at /home/camaud/programming/genepi/node_modules/node-addon-api/napi-inl.h:2951 #6 0x00007fffdfdde834 in genepi::ClassWrapper<Test>::Initialize (env=..., target=..., name="Test", static_methodList=std::deque with 1 element = {...}, methodList=std::deque with 3 elements = {...}) at /home/camaud/programming/genepi/include/genepi/class_wrapper.h:38 #7 0x00007fffdfdde406 in genepi::BindClass<Test>::initialize ( this=0x7fffdffff960 <genepi::BindClass<Test>::getInstance()::instance>, env=..., target=...) at /home/camaud/programming/genepi/include/genepi/bind_class.h:32 #8 0x00007fffdfde5f06 in initialize (env=..., exports=...) at /home/camaud/programming/genepi/src/genepi/genepi.cpp:3 #9 0x00007fffdfde623f in Napi::RegisterModule(napi_env__*, napi_value__*, Napi::Object (*)(Napi::Env, Napi::Object))::{lambda()#1}::operator()() const ( __closure=0x7fffffffbb50) at /home/camaud/programming/genepi/node_modules/node-addon-api/napi-inl.h:222 #10 0x00007fffdfde62c4 in Napi::details::WrapCallback<Napi::RegisterModule(napi_env__*, napi_value__*, Napi::Object (*)(Napi::Env, Napi::Object))::{lambda()#1}>(Napi::RegisterModule(napi_env__*, napi_value__*, Napi::Object (*)(Napi::Env, Napi::Object))::{lambda()#1}) (callback=...) at /home/camaud/programming/genepi/node_modules/node-addon-api/napi-inl.h:104 #11 0x00007fffdfde62ac in Napi::RegisterModule (env=0x2541500, exports=0x251df18, registerCallback=0x7fffdfde5cf2 <initialize(Napi::Env, Napi::Object)>) at /home/camaud/programming/genepi/node_modules/node-addon-api/napi-inl.h:224 #12 0x00007fffdfde5f6c in __napi_initialize (env=0x2541500, exports=0x251df18) at /home/camaud/programming/genepi/src/genepi/genepi.cpp:3 #13 0x00000000008e14a1 in napi_module_register_by_symbol(v8::Local<v8::Object>, v8::Local<v8::Value>, v8::Local<v8::Context>, napi_value__* (*)(napi_env__*, napi_value__*)) () #14 0x00000000008d3352 in node::DLOpen(v8::FunctionCallbackInfo<v8::Value> const&) () #15 0x0000000000b8b32f in v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) () #16 0x0000000000b8be99 in v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) ()The code is not open yet but if we cannot find the issue, I will send some files to you.
I read the commit
[917bd60] - src: remove TODOs by fixing memory leaks (Gabriel Schulhof) #343and I see now why my code does not work anymore.template < class Bound > class ClassWrapper : public Napi::ObjectWrap< ClassWrapper< Bound > > { public: using Wrapper = ClassWrapper< Bound >; using Descriptor = typename Wrapper::PropertyDescriptor; ClassWrapper( const Napi::CallbackInfo& info ); static void Initialize( Napi::Env& env, Napi::Object& target, const std::string& name, const std::deque< MethodDef >& static_methodList, const std::deque< MethodDef >& methodList ) { std::vector< Descriptor > descriptors; descriptors.reserve( static_methodList.size() + methodList.size() ); add_methods( env, static_methodList, descriptors ); add_methods( env, methodList, descriptors ); for( unsigned int i = static_methodList.size(); i < descriptors.size(); i++ ) { auto& attr = static_cast< napi_property_descriptor& >( descriptors[i] ) .attributes; attr = static_cast< napi_property_attributes >( attr & ~napi_static ); } auto function = Wrapper::DefineClass( env, name.c_str(), descriptors ); constructor_ = Napi::Persistent( function ); constructor_.SuppressDestruct(); target.Set( name.c_str(), function ); } static Napi::Object create( Napi::Env env, const std::vector< napi_value >& args ) { Napi::EscapableHandleScope scope( env ); return scope.Escape( constructor_.New( args ) ).ToObject(); } template < typename... Args > static void create_obj( const Napi::CallbackInfo& info, Args&&... args ) { Wrapper::get_smartpointer( info.This() ) .reset( new Bound{ args... } ); } static Bound* get_bound( const Napi::CallbackInfo& info ) { return Wrapper::get_smartpointer( info.This() ).get(); } static Bound* get_bound( const Napi::Value& arg ) { return Wrapper::get_smartpointer( arg ).get(); } private: static std::shared_ptr< Bound >& get_smartpointer( const Napi::Value& value ) { return Wrapper::Unwrap( value.ToObject() )->underlying_class_; } static void add_methods( Napi::Env& env, const std::deque< MethodDef >& methodList, std::vector< Descriptor >& descriptors ) { for( const auto& method : methodList ) { auto* method_param = new genepi::SignatureParam; method_param->methodNum = method.getNum(); descriptors.emplace_back( Wrapper::StaticMethod( method.getName().c_str(), method.getSignature()->getCaller(), napi_default, static_cast< void* >( Napi::External< genepi::SignatureParam >::New( env, method_param ) .Data() ) ) ); } } private: static Napi::FunctionReference constructor_; std::shared_ptr< Bound > underlying_class_; };The idea is to have an automatic wrapper arround a custom class
Bound. I have the list of all static function and method pointers. I want to hook to these pointers to theClassWrapperinstance.The trick is that the methods are not defined in the
ClassWrapper, they are defined as classic C++ functions. So I can only use yourStaticMethodhelper function but I want to use them as a method from the JS side. Removing thenapi_staticflag does the trick.To sum up (because I am not sure to be clear...), I want a
StaticMethodof Napi but a classic method from JS side. I think this trick can no longer be done with your new implementation ofDefineClass.@BotellaA Each function and accessor you define causes the wrapper to allocate data on the heap. This data has to be deleted when it is clear that the accessor or the function will never be called again. It will usually never be called again when it has been garbage-collected. In the case of JS classes this means that the data associated with accessors and prototype methods will be deleted when the JS class itself is garbage-collected, and in the case of static methods the data will be deleted when the static methods are garbage collected, because the static methods can be detached from the class and thus may outlive the class. To reflect this, the new implementation converts static method descriptions to
Napi::Functions before callingnapi_define_class, and attaches the heap-allocated data to theNapi::Function.So the reason this does not work anymore is that V8 crashes when attempting to attach a ready-made
Napi::Functionto the JS class prototype.@gabrielschulhof Ok, I understand the need to create
Napi::Functions but not really how to make this design to work... Any recommendation?@BotellaA you can always reach down into N-API for custom behaviour:
@gabrielschulhof Thanks, I will look into this. Since the error is not on your side, I close the issue.
I update to node-addon-api 1.5.0 (from 1.4.0) and I got this error at runtime using gdb
and this stack
Do you have an idea why this crashes in 1.5.0 and not in 1.4.0? I had the same crash on several computers with several gcc and node versions. Any major changes in Napi::ObjectWrap::DefineClass?
Thanks for your help!