objectwrap: avoid double-free on old Node.js

On Node.js versions earlier than 10.15.3 references were being cleaned
up differently. This has resulted in a segfault because of a double
free when the `ObjectWrap<T>` subclass constructor throws. We need a
fix that only runs on Node.js < 10.15.3. We use
`Napi::VersionManagement::GetNodeVersion()` at addon load time to
determine whether we need the workaround. We store the result in an
atomic boolean and consult its value whenever we destroy an
`ObjectWrap<T>` instance.

Author: Gabriel Schulhof

napi-inl.h

@@ -10,6 +10,7 @@
 // Note: Do not include this file directly! Include "napi.h" instead.
 
 #include <algorithm>
+#include <atomic>
 #include <cstring>
 #include <mutex>
 #include <type_traits>
@@ -19,6 +20,8 @@ namespace Napi {
 // Helpers to handle functions exposed from C++.
 namespace details {
 
+extern std::atomic_bool needs_objectwrap_destructor_fix;
+
 // Attach a data item to an object and delete it when the object gets
 // garbage-collected.
 // TODO: Replace this code with `napi_add_finalizer()` whenever it becomes
@@ -251,11 +254,16 @@ struct AccessorCallbackData {
 // Module registration
 ////////////////////////////////////////////////////////////////////////////////
 
-#define NODE_API_MODULE(modname, regfunc)                 \
-  napi_value __napi_ ## regfunc(napi_env env,             \
-                                napi_value exports) {     \
-    return Napi::RegisterModule(env, exports, regfunc);   \
-  }                                                       \
+#define NODE_API_MODULE(modname, regfunc)                      \
+  namespace Napi {                                             \
+    namespace details {                                        \
+      std::atomic_bool needs_objectwrap_destructor_fix(false); \
+    }                                                          \
+  }                                                            \
+  napi_value __napi_ ## regfunc(napi_env env,                  \
+                                napi_value exports) {          \
+    return Napi::RegisterModule(env, exports, regfunc);        \
+  }                                                            \
   NAPI_MODULE(modname, __napi_ ## regfunc)
 
 // Adapt the NAPI_MODULE registration function:
@@ -264,6 +272,12 @@ struct AccessorCallbackData {
 inline napi_value RegisterModule(napi_env env,
                                  napi_value exports,
                                  ModuleRegisterCallback registerCallback) {
+  const napi_node_version* nver = Napi::VersionManagement::GetNodeVersion(env);
+  Napi::details::needs_objectwrap_destructor_fix =
+    (nver->major < 10 ||
+      (nver->major == 10 && nver->minor < 15) ||
+      (nver->major == 10 && nver->minor == 15 && nver->patch < 3));
+
   return details::WrapCallback([&] {
     return napi_value(registerCallback(Napi::Env(env),
                                        Napi::Object(env, exports)));
@@ -2985,6 +2999,16 @@ inline ObjectWrap<T>::~ObjectWrap() {
     // This happens e.g. during garbage collection.
     if (!object.IsEmpty() && _construction_failed) {
       napi_remove_wrap(Env(), object, nullptr);
+
+      if (Napi::details::needs_objectwrap_destructor_fix) {
+        // If construction failed we delete the reference via
+        // `napi_remove_wrap()`, not via `napi_delete_reference()` in the
+        // `Reference<Object>` destructor. This will prevent the
+        // `Reference<Object>` destructor from doing a double delete of this
+        // reference.
+        _ref = nullptr;
+        _env = nullptr;
+      }
     }
   }
 }

test/objectwrap_constructor_exception.cc

@@ -5,6 +5,7 @@ class ConstructorExceptionTest :
 public:
   ConstructorExceptionTest(const Napi::CallbackInfo& info) :
     Napi::ObjectWrap<ConstructorExceptionTest>(info) {
+    fprintf(stderr, "That was the interesting reference\n");
     Napi::Error error = Napi::Error::New(info.Env(), "an exception");
 #ifdef NAPI_DISABLE_CPP_EXCEPTIONS
     error.ThrowAsJavaScriptException();