Reference count of ObjectWrap changed in Node.js 14.17.0

[oyyd]

Not sure if this is an issue of napi or node-addon-api, but we found this when using node-addon-api so I post it here.

The code below:

#include <napi.h>

class MyObject : public Napi::ObjectWrap<MyObject> {
 public:
  static Napi::Object Init(Napi::Env env, Napi::Object exports);
  MyObject(const Napi::CallbackInfo& info);
  ~MyObject();
};

Napi::Object MyObject::Init(Napi::Env env, Napi::Object exports) {
  Napi::Function func =
      DefineClass(env,
                  "MyObject",
                  {});

  Napi::FunctionReference* constructor = new Napi::FunctionReference();
  *constructor = Napi::Persistent(func);
  env.SetInstanceData(constructor);

  exports.Set("MyObject", func);
  return exports;
}

MyObject::MyObject(const Napi::CallbackInfo& info)
    : Napi::ObjectWrap<MyObject>(info) {
  this->Ref(); // Nothing special except that we Ref() here to keep this object from been garbage collected while other resources still alive
}

MyObject::~MyObject() {
  uint32_t count = -1; 
  count = this->Unref(); // And Unref() in destructor.
  fprintf(stderr, "count %d\n", count); // Log the count.
}

Napi::Object InitAll(Napi::Env env, Napi::Object exports) {
  return MyObject::Init(env, exports);
}

NODE_API_MODULE(addon, InitAll)

var addon = require('bindings')('addon');

new addon.MyObject(10); // Create an instance and wait the process to exit.

Running the code in Node.js 14.16 will get:

count 0

In Node.js 14.17, the result becomes:

count 1

Though the code snippet doesn't throw here, in our real programs, running similar codes in Node.js 14.17 causes:

When enable cpp exceptions i.e. NAPI_CPP_EXCEPTIONS, run the script will result in:

// Mac
libc++abi.dylib: terminating with uncaught exception of type Napi::Error
// Debian
terminate called after throwing an instance of 'Napi::Error'
  what():  Unknown failure
Aborted (core dumped)

[legendecas]

@oyyd can you share the core dump backtrace in your realm programs? (you can wipe out sensitive data from the text backtrace) It will be helpful to track down the problem since you said the code snippet doesn't crash.

[oyyd]

@legendecas The backtrace is:

(lldb) bt
* thread #1, name = 'node', stop reason = signal SIGABRT
  * frame #0: libc.so.6`__GI_raise(sig=6) at raise.c:56
    frame #1: libc.so.6`__GI_abort at abort.c:89
    frame #2: 0x00007f7030e2cb5d libstdc++.so.6`__gnu_cxx::__verbose_terminate_handler() + 349
    frame #3: 0x00007f7030e2abb6 libstdc++.so.6`___lldb_unnamed_symbol46$$libstdc++.so.6 + 6
    frame #4: 0x00007f7030e29ca9 libstdc++.so.6`___lldb_unnamed_symbol32$$libstdc++.so.6 + 57
    frame #5: 0x00007f7030e2a40b libstdc++.so.6`__gxx_personality_v0 + 235
    frame #6: 0x00007f70308c4ff3 libgcc_s.so.1`___lldb_unnamed_symbol15$$libgcc_s.so.1 + 67
    frame #7: 0x00007f70308c537b libgcc_s.so.1`_Unwind_RaiseException + 251
    frame #8: 0x00007f7030e2ae2c libstdc++.so.6`__cxa_throw + 92
    frame #9: 0x00007f702d8db03b unix_dgram.node`SocketAddon::UnixDgramSocket::UnrefAll() + 155
    frame #10: 0x00007f702d8e2a0d unix_dgram.node`Napi::ObjectWrap<SocketAddon::UnixDgramSocket>::FinalizeCallback(napi_env__*, void*, void*) + 29
    frame #11: 0x00007ffd769c32c0 node`v8impl::(anonymous namespace)::Reference::Finalize(bool) + 192
    frame #12: 0x00007ffd769e235f node`node_napi_env__::~node_napi_env__() + 95
    frame #13: 0x00007ffd769b422b node`node::Environment::RunCleanup() + 619
    frame #14: 0x00007ffd76970497 node`node::FreeEnvironment(node::Environment*) + 71
    frame #15: 0x00007ffd76a4d4bf node`node::NodeMainInstance::Run() + 271
    frame #16: 0x00007ffd769dbba5 node`node::Start(int, char**) + 277
    frame #17: libc.so.6`__libc_start_main(main=(node`main), argc=2, argv=0x00007ffd7629e148, init=<unavailable>, fini=<unavailable>, rtld_fini=<unavailable>, stack_end=0x00007ffd7629e138) at libc-start.c:287
    frame #18: 0x00007ffd7696b80c node`_start + 41

Although I didn't build a debug version of Node.js to inspect the coredump file, the code that aborts inside UnixDgramSocket::UnrefAll() is this->Unref(); which was found by changing of the code.

[oyyd]

@legendecas I forgot to enable cpp exceptions. The code snippet also aborts when cpp exceptions enabled.

[legendecas]

I believe it is this commit nodejs/node@03806a0#diff-4b35cbf831be523a4f3e5aac3729c95490ed220892695d0e4245c15c1f63cfc1R278 making the problem prominent.

Although this crash is introduced in a newer version of Node.js, the first impression came to me that this is a user code bug in terms of API semantics. Calling napi_reference_unref/Reference<T>::Unref in ObjectWraps destructor should be avoided as the destructor is trigger by garbage collection, which means the object's ref count is already 0. @gabrielschulhof maybe you can elaborate on the error returning behavior https://github.com/nodejs/node/blob/master/src/js_native_api_v8.cc#L2523-L2526? Anyway, it is meaningless when the ref count goes down to negative values.

The value 1 returned in Node.js v14.7 and later is from the default value defined here https://github.com/nodejs/node-addon-api/blob/main/napi-inl.h#L2594. Maybe returning 0 on non-napi_ok status is more reasonable?

[oyyd]

I call Unref() in destructor (or finalize callbacks) or it will also result in aborting:

#include <napi.h>

class MyObject : public Napi::ObjectWrap<MyObject> {
 public:
  static Napi::Object Init(Napi::Env env, Napi::Object exports);
  MyObject(const Napi::CallbackInfo& info);
  ~MyObject();
};

Napi::Object MyObject::Init(Napi::Env env, Napi::Object exports) {
  Napi::Function func =
      DefineClass(env,
                  "MyObject",
                  {});

  Napi::FunctionReference* constructor = new Napi::FunctionReference();
  *constructor = Napi::Persistent(func);
  env.SetInstanceData(constructor);

  exports.Set("MyObject", func);
  return exports;
}

MyObject::MyObject(const Napi::CallbackInfo& info)
    : Napi::ObjectWrap<MyObject>(info) {
  this->Ref(); 
}

MyObject::~MyObject() {
  // comment these lines
  // uint32_t count = -1; 
  // count = this->Unref(); 
  // fprintf(stderr, "count %d\n", count); 
}

Napi::Object InitAll(Napi::Env env, Napi::Object exports) {
  return MyObject::Init(env, exports);
}

NODE_API_MODULE(addon, InitAll)

get:

libc++abi.dylib: terminating with uncaught exception of type Napi::Error

[legendecas]

@oyyd can you share the build configuration and Node.js version when not calling Unref in ObjectWrap subclass destructors? I cannot reproduce the problem either when c++ exceptions are enabled or not locally.

[oyyd]

@legendecas Please see this example https://github.com/oyyd/addon_abort_example. Thanks for your patient investigation:)

[legendecas]

@oyyd sorry about the delay but your example did call Unref in the destructor. I comment out those lines Unref in MyObject::~MyObject and in Node.js v14.17.1 it run alright.

[github-actions]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.