Add N-API compliance tests

.github/workflows/build-linux.yml

@@ -65,3 +65,9 @@ jobs:
           # thread_suspend() and is unaffected.
           JSC_useConcurrentGC: ${{ inputs.enable-thread-sanitizer && '0' || '' }}
 
+      - name: Run Node-API v5 conformance
+        working-directory: Build/ubuntu/Tests/NodeApi
+        run: ./NodeApiTests
+        env:
+          TSAN_OPTIONS: ${{ inputs.enable-thread-sanitizer && format('suppressions={0}/.github/tsan_suppressions.txt', github.workspace) || '' }}
+          JSC_useConcurrentGC: ${{ inputs.enable-thread-sanitizer && '0' || '' }}

.github/workflows/build-macos.yml

@@ -41,11 +41,16 @@ jobs:
             -D ENABLE_THREAD_SANITIZER=${{ inputs.enable-thread-sanitizer && 'ON' || 'OFF' }}
 
       - name: Build
-        run: cmake --build Build/macOS --target UnitTests --config RelWithDebInfo
+        run: cmake --build Build/macOS --target UnitTests NodeApiTests --config RelWithDebInfo
 
       - name: Run Tests
         working-directory: Build/macOS/Tests/UnitTests/RelWithDebInfo
         run: ./UnitTests
         env:
           TSAN_OPTIONS: ${{ inputs.enable-thread-sanitizer && format('suppressions={0}/.github/tsan_suppressions.txt', github.workspace) || '' }}
 
+      - name: Run Node-API v5 conformance
+        working-directory: Build/macOS/Tests/NodeApi/RelWithDebInfo
+        run: ./NodeApiTests
+        env:
+          TSAN_OPTIONS: ${{ inputs.enable-thread-sanitizer && format('suppressions={0}/.github/tsan_suppressions.txt', github.workspace) || '' }}

CMakeLists.txt

@@ -154,6 +154,17 @@ endif()
 FetchContent_MakeAvailable_With_Message(arcana.cpp)
 set_property(TARGET arcana PROPERTY FOLDER Dependencies)
 
+if(ANDROID)
+    FetchContent_GetProperties(AndroidExtensions)
+    if(NOT AndroidExtensions_POPULATED)
+        FetchContent_Populate(AndroidExtensions)
+        FetchContent_GetProperties(AndroidExtensions)
+        add_subdirectory(${androidextensions_SOURCE_DIR} ${androidextensions_BINARY_DIR})
+    else()
+        add_subdirectory(${androidextensions_SOURCE_DIR} ${androidextensions_BINARY_DIR})
+    endif()
+endif()
+
 if(JSRUNTIMEHOST_POLYFILL_XMLHTTPREQUEST OR JSRUNTIMEHOST_POLYFILL_FETCH)
     FetchContent_MakeAvailable_With_Message(UrlLib)
     set_property(TARGET UrlLib PROPERTY FOLDER Dependencies)

Core/Node-API-JSI/Include/napi/napi.h

@@ -1,49 +1,14 @@
 #pragma once
 
 #include <jsi/jsi.h>
+#include <napi/js_native_api_types.h>
 #include <functional>
 #include <initializer_list>
 #include <memory>
 #include <string>
 #include <vector>
 #include <optional>
 
-// Copied from js_native_api_types.h (https://git.io/J8aI5)
-typedef enum {
-  napi_default = 0,
-  napi_writable = 1 << 0,
-  napi_enumerable = 1 << 1,
-  napi_configurable = 1 << 2,
-} napi_property_attributes;
-
-typedef enum {
-  // ES6 types (corresponds to typeof)
-  napi_undefined,
-  napi_null,
-  napi_boolean,
-  napi_number,
-  napi_string,
-  napi_symbol,
-  napi_object,
-  napi_function,
-  napi_external,
-} napi_valuetype;
-
-typedef enum {
-  napi_int8_array,
-  napi_uint8_array,
-  napi_uint8_clamped_array,
-  napi_int16_array,
-  napi_uint16_array,
-  napi_int32_array,
-  napi_uint32_array,
-  napi_float32_array,
-  napi_float64_array,
-  // JSI doesn't support bigint.
-  // napi_bigint64_array,
-  // napi_biguint64_array,
-} napi_typedarray_type;
-
 struct napi_env__ {
   napi_env__(facebook::jsi::Runtime& rt)
     : rt{rt}

Core/Node-API/CMakeLists.txt

@@ -7,7 +7,10 @@ elseif(ANDROID)
     set(NAPI_JAVASCRIPT_ENGINE "V8" CACHE STRING "JavaScript engine for Node-API")
 elseif(UNIX)
     set(NAPI_JAVASCRIPT_ENGINE "JavaScriptCore" CACHE STRING "JavaScript engine for Node-API")
-    set(JAVASCRIPTCORE_LIBRARY "/usr/lib/x86_64-linux-gnu/libjavascriptcoregtk-4.1.so" CACHE STRING "Path to the JavaScriptCore shared library")
+    find_library(JAVASCRIPTCORE_LIBRARY javascriptcoregtk-4.1)
+    if(NOT JAVASCRIPTCORE_LIBRARY)
+        message(FATAL_ERROR "JavaScriptCore library not found. Please install libwebkit2gtk-4.1-dev")
+    endif()
 else()
     message(FATAL_ERROR "Unable to select Node-API JavaScript engine for platform")
 endif()
@@ -261,7 +264,24 @@ Make sure Hermes was fetched at the top-level CMakeLists.txt and NAPI_JAVASCRIPT
     message(STATUS "Selected ${NAPI_JAVASCRIPT_ENGINE}")
 endif()
 
-add_library(napi ${SOURCES})
+# On Android, native addons are dlopen'd as standalone .node modules and resolve their napi_* imports
+# from a shared napi at load time -- bionic will not surface a statically-linked host's napi to a
+# dlopen'd module, so the host and every addon must share a single libnapi.so. Default napi to a
+# shared library on Android so that model works out of the box; an integrator who wants a static napi
+# (e.g. for size/packaging) can override with -DJSR_NAPI_SHARED=OFF. The option defaults OFF on other
+# platforms, where napi keeps following the project's default library type (i.e. honors
+# BUILD_SHARED_LIBS).
+set(JSR_NAPI_SHARED_DEFAULT OFF)
+if(ANDROID)
+    set(JSR_NAPI_SHARED_DEFAULT ON)
+endif()
+option(JSR_NAPI_SHARED "Build napi as a shared library (libnapi.so)" ${JSR_NAPI_SHARED_DEFAULT})
+
+if(JSR_NAPI_SHARED)
+    add_library(napi SHARED ${SOURCES})
+else()
+    add_library(napi ${SOURCES})
+endif()
 
 target_include_directories(napi ${INCLUDE_DIRECTORIES})
 target_link_libraries(napi ${LINK_LIBRARIES})

Core/Node-API/Source/js_native_api_javascriptcore.cc

@@ -446,7 +446,13 @@ namespace {
         return napi_set_last_error(env, napi_generic_failure);
       }
 
-      JSObjectRef function{JSObjectMakeFunctionWithCallback(env->context, JSString(utf8name), CallAsFunction)};
+      // `length` is a byte count and the name need not be null-terminated.
+      // The Node-API CTS deliberately passes "Name_extra" with length 5 and
+      // expects Function#name to be "Name_".
+      JSString functionName{
+        utf8name != nullptr ? utf8name : "",
+        utf8name != nullptr ? length : 0};
+      JSObjectRef function{JSObjectMakeFunctionWithCallback(env->context, functionName, CallAsFunction)};
       JSObjectRef sentinel{JSObjectMake(env->context, info->_class, info)};
       CHECK_NAPI(NativeInfo::Link<FunctionInfo>(env, function, sentinel));
 
@@ -561,16 +567,29 @@ namespace {
     static void Finalize(JSObjectRef object) {
       T* info = Get<T>(object);
       assert(info->Type() == TType);
-      for (const FinalizerT& finalizer : info->_finalizers) {
-        finalizer(info);
+      if (info->Env()->defer_finalizer_if_requested([info]() {
+            RunFinalizers(info);
+          })) {
+        // JSC invokes JSClass finalizers while the heap is collecting, where
+        // re-entering JavaScript is unsafe. Node-API finalizers are allowed to
+        // call back through Node-API, so the explicit test-GC path drains them
+        // immediately after the synchronous collection returns.
+        return;
       }
-      delete info;
+      RunFinalizers(info);
     }
 
     napi_env _env;
     void* _data{};
     std::vector<FinalizerT> _finalizers{};
     JSClassRef _class{};
+
+    static void RunFinalizers(T* info) {
+      for (const FinalizerT& finalizer : info->_finalizers) {
+        finalizer(info);
+      }
+      delete info;
+    }
   };
 
   class ExternalInfo: public BaseInfoT<ExternalInfo, NativeType::External> {
@@ -666,10 +685,20 @@ namespace {
       return napi_ok;
     }
 
+    bool IsWrapped() const {
+      return _wrapped;
+    }
+
+    void IsWrapped(bool wrapped) {
+      _wrapped = wrapped;
+    }
+
    private:
     WrapperInfo(napi_env env)
       : BaseInfoT{env, "Native (Wrapper)"} {
     }
+
+    bool _wrapped{};
   };
 
   class ExternalArrayBufferInfo {
@@ -740,6 +769,9 @@ struct napi_ref__ {
     CHECK_NAPI(ReferenceInfo::GetObjectId(env, _value, &_objectId));
     if (_objectId == 0) {
       CHECK_NAPI(ReferenceInfo::Initialize(env, _value, [value = _value](ReferenceInfo* info) {
+        if (info->Env()->shutting_down) {
+          return;
+        }
         auto entry{info->Env()->active_ref_values.find(value)};
         // NOTE: The finalizer callback is actually on a "sentinel" JS object that is linked to the
         // actual JS object we are trying to track. This means it is possible for the tracked object
@@ -832,6 +864,20 @@ void napi_env__::deinit_refs() {
   }
 }
 
+void napi_env__::delete_remaining_refs() {
+  // Addons may delete wrap-owned references from their finalizers while
+  // JSGlobalContextRelease tears down the heap. Keep every reference tracked
+  // until that teardown has run, then reclaim only the references the addon
+  // left behind. Deleting strong references before context teardown would
+  // make a finalizer's napi_delete_reference double-free them; never deleting
+  // the leftovers leaks references whose finalizer deliberately does not.
+  assert(strong_refs.empty());
+  for (napi_ref ref : refs) {
+    delete ref;
+  }
+  refs.clear();
+}
+
 void napi_env__::init_symbol(JSValueRef &symbol, const char *description) {
   symbol = JSValueMakeSymbol(context, JSString(description));
   JSValueProtect(context, symbol);
@@ -841,6 +887,18 @@ void napi_env__::deinit_symbol(JSValueRef symbol) {
   JSValueUnprotect(context, symbol);
 }
 
+void napi_env__::init_function_prototype_call() {
+  // Capture the canonical Function.prototype.call once, at env init, so napi_call_function does not
+  // depend on a target function's own (user-overridable) "call" property.
+  JSObjectRef global = JSContextGetGlobalObject(context);
+  JSValueRef function_ctor = JSObjectGetProperty(context, global, JSString("Function"), nullptr);
+  JSObjectRef function_ctor_obj = JSValueToObject(context, function_ctor, nullptr);
+  JSValueRef prototype = JSObjectGetProperty(context, function_ctor_obj, JSString("prototype"), nullptr);
+  JSObjectRef prototype_obj = JSValueToObject(context, prototype, nullptr);
+  function_prototype_call = JSObjectGetProperty(context, prototype_obj, JSString("call"), nullptr);
+  JSValueProtect(context, function_prototype_call);
+}
+
 // Warning: Keep in-sync with napi_status enum
 static const char* error_messages[] = {
   nullptr,
@@ -893,6 +951,7 @@ napi_status napi_create_function(napi_env env,
                                  void* callback_data,
                                  napi_value* result) {
   CHECK_ENV(env);
+  CHECK_ARG(env, cb);
   CHECK_ARG(env, result);
 
   CHECK_NAPI(FunctionInfo::Create(env, utf8name, length, cb, callback_data, result));
@@ -1642,14 +1701,27 @@ napi_status napi_call_function(napi_env env,
     CHECK_ARG(env, argv);
   }
 
+  JSObjectRef function_object = ToJSObject(env, func);
+
+  std::vector<JSValueRef> call_args(argc + 1);
+  call_args[0] = ToJSValue(recv);
+  for (size_t i = 0; i < argc; ++i) {
+    call_args[i + 1] = ToJSValue(argv[i]);
+  }
+
   JSValueRef exception{};
-  JSValueRef return_value{JSObjectCallAsFunction(
-    env->context,
-    ToJSObject(env, func),
-    JSValueIsUndefined(env->context, ToJSValue(recv)) ? nullptr : ToJSObject(env, recv),
-    argc,
-    ToJSValues(argv),
-    &exception)};
+  // Invoke through the canonical Function.prototype.call (captured at env init), not the target's own
+  // "call" property -- user code could override func.call and change native call behavior.
+  JSObjectRef call_object =
+      JSValueToObject(env->context, env->function_prototype_call, &exception);
+  CHECK_JSC(env, exception);
+
+  JSValueRef return_value{JSObjectCallAsFunction(env->context,
+                                                 call_object,
+                                                 function_object,
+                                                 call_args.size(),
+                                                 call_args.data(),
+                                                 &exception)};
   CHECK_JSC(env, exception);
 
   if (result != nullptr) {
@@ -1966,8 +2038,9 @@ napi_status napi_wrap(napi_env env,
 
   WrapperInfo* info{};
   CHECK_NAPI(WrapperInfo::Wrap(env, js_object, &info));
-  RETURN_STATUS_IF_FALSE(env, info->Data() == nullptr, napi_invalid_arg);
+  RETURN_STATUS_IF_FALSE(env, !info->IsWrapped(), napi_invalid_arg);
 
+  info->IsWrapped(true);
   info->Data(native_object);
 
   if (finalize_cb != nullptr) {
@@ -1989,7 +2062,7 @@ napi_status napi_unwrap(napi_env env, napi_value js_object, void** result) {
 
   WrapperInfo* info{};
   CHECK_NAPI(WrapperInfo::Unwrap(env, js_object, &info));
-  RETURN_STATUS_IF_FALSE(env, info != nullptr && info->Data() != nullptr, napi_invalid_arg);
+  RETURN_STATUS_IF_FALSE(env, info != nullptr && info->IsWrapped(), napi_invalid_arg);
 
   *result = info->Data();
   return napi_ok;
@@ -1999,17 +2072,16 @@ napi_status napi_remove_wrap(napi_env env, napi_value js_object, void** result)
   CHECK_ENV(env);
   CHECK_ARG(env, js_object);
 
-  // REVIEW: Should we remove the wrapper if we are removing finalizers anyway?
-
   WrapperInfo* info{};
   CHECK_NAPI(WrapperInfo::Unwrap(env, js_object, &info));
-  RETURN_STATUS_IF_FALSE(env, info != nullptr && info->Data() != nullptr, napi_invalid_arg);
+  RETURN_STATUS_IF_FALSE(env, info != nullptr && info->IsWrapped(), napi_invalid_arg);
 
   if (result)
   {
     *result = info->Data();
   }
 
+  info->IsWrapped(false);
   info->Data(nullptr);
   info->RemoveFinalizers();
 
@@ -2047,12 +2119,35 @@ napi_status napi_create_reference(napi_env env,
   CHECK_ARG(env, value);
   CHECK_ARG(env, result);
 
+  // References are backed by metadata attached to a JavaScript object.
+  // Reject unsupported primitives before ReferenceInfo calls ToJSObject():
+  // that conversion asserts, while node-addon-api deliberately probes this
+  // API and wraps a non-object pending exception when it is rejected.
+  //
+  // This is especially important for JavaScriptCore's watchdog termination
+  // exception. WebKit represents it as the string "JavaScript execution
+  // terminated.", so attempting to persist it as an object would abort the
+  // host while stopping a tight-loop Worker.
+  // https://github.com/WebKit/WebKit/blob/46327f724be09f4b27c1c72b9dd083ca34d6abcc/Source/JavaScriptCore/API/tests/ExecutionTimeLimitTest.cpp
+  napi_valuetype value_type{};
+  CHECK_NAPI(napi_typeof(env, value, &value_type));
+  if (value_type != napi_object &&
+      value_type != napi_function &&
+      value_type != napi_external) {
+    return napi_set_last_error(env, napi_object_expected);
+  }
+
   napi_ref__* ref{new napi_ref__{}};
   if (ref == nullptr) {
     return napi_set_last_error(env, napi_generic_failure);
   }
 
-  ref->init(env, value, initial_refcount);
+  const napi_status status{ref->init(env, value, initial_refcount)};
+  if (status != napi_ok) {
+    delete ref;
+    return status;
+  }
+  env->track_ref(ref);
   *result = ref;
 
   return napi_ok;
@@ -2064,6 +2159,7 @@ napi_status napi_delete_reference(napi_env env, napi_ref ref) {
   CHECK_ENV(env);
   CHECK_ARG(env, ref);
 
+  env->untrack_ref(ref);
   ref->deinit(env);
   delete ref;