Hi, using latest version from git, I'm getting crashes when node is closing about invalid access to memory or even double-free of the same pointer. I compiled node 13.7.0 with debug and asan to get a full report of the crash and that's the output: see gist.

For what I found running the debugger:

• When an object is freed in this scenario, the ObjectWrap destructor runs the napi_remove_wrap function. This function calls another inside the NAPI which does not remove the reference yet, but sets a variable to true (see v8impl::Reference::Delete).
• Then, as ObjectWrap inherits from Reference, the Reference destructor calls the function napi_delete_reference, in which the NAPI internally runs again the same Delete function from before but this time deleting the object (see code).
• All of above is running because node is running the finalizer for the wrap (I guess) here.
• After running the finalizer, then it tries to continue doing its job to see if v8impl::Reference::Delete should be called or not, but as the reference (this) itself has been deleted already, asan complains and shows the above report (see code).

I replicated the crash under 12.14.1 and 13.7.0 but not on 10.18.1 using the latest master code of this package (commit 4648420) all on macOS. The crash also replicates for any of the objects I create from JS which is a ObjectWrap.

While doing some changes to try to avoid the crash, I come up with adding this->SuppressDestruct(); when the napi_remove_wrap is called in the ObjectWrap destructor:

template <typename T>
inline ObjectWrap<T>::~ObjectWrap() {
  // If the JS object still exists at this point, remove the finalizer added
  // through `napi_wrap()`.
  if (!IsEmpty()) {
    Object object = Value();
    // It is not valid to call `napi_remove_wrap()` with an empty `object`.
    // This happens e.g. during garbage collection.
    if (!object.IsEmpty()) {
      napi_remove_wrap(Env(), object, nullptr);
      this->SuppressDestruct(); //I added this line so the reference is not deleted yet
    }
  }
}

I think this is the worst way to fix it, but at least I don't get crashes.

Edit: the call-to-SuppressDestruct() trick sometimes does not work if the constructor of the C++ object throws an exception.