node-api: enable napi_ref for all value types

vmoroz · Oct 10, 2022 · Oct 10, 2022 · Oct 28, 2022 · Oct 28, 2022 · Nov 4, 2022
commit 579d44cfbe994499f72308c8893829cea5b9b720
diff --git a/doc/api/n-api.md b/doc/api/n-api.md
@@ -2046,6 +2046,48 @@ the `napi_value` in question is of the JavaScript type expected by the API.
 
 ### Enum types
 
+#### `napi_features`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+> Stability: 1 - Experimental
+
+```c
+typedef enum {
+  napi_feature_none = 0,
+  napi_feature_reference_all_types = 1 << 0,
+
+  napi_default_experimental_features = napi_feature_reference_all_types,
+
+  napi_default_features = napi_default_experimental_features, // version specific
+} napi_features;
+```
+
+The `napi_features` allow changing internal behavior of existing Node-API
+functions.
+
+We pass a `napi_features` pointer to the `napi_module` struct in the
+`NAPI_MODULE_X` macro. This macro is used for the module registration.
+If the module is initialized without using this macro, then there will be
+no features selected and the module will use the `napi_feature_none`.
+
+Each Node-API version defines its own default set of features.
+For the current version it can be accessed using `napi_default_features`.
+A module can override the set of its enabled features by adding
+`NAPI_CUSTOM_FEATURES` definition to the `.gyp` file and then defining the
+value of the global `napi_module_features` variable.
+To check enabled features use the `napi_is_feature_enabled` function.
+
+For example, to disables `napi_feature_reference_all_types` feature we can
+exclude its bit from the `napi_default_features` set:
+
+```c
+napi_features napi_module_features =
+    napi_default_features & ~napi_feature_reference_all_types;
+```
+
 #### `napi_key_collection_mode`
 
 <!-- YAML
@@ -5700,6 +5742,32 @@ support it:
 * If the function is not available, provide an alternate implementation
   that does not use the function.
 
+#### `napi_is_feature_enabled`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+> Stability: 1 - Experimental
+
+```c
+NAPI_EXTERN napi_status napi_is_feature_enabled(napi_env env,
+                                                napi_features feature,
+                                                bool* result);
+```
+
+* `[in] env`: The environment that the API is invoked under.
+* `[in] feature`: The feature that we want to test.
-* `[in] feature`: The feature that we want to test.
+* `[in] feature`: The features that we want to test.
-* `[in] feature`: The feature that we want to test.
+* `[in] feature`: The features that we want to test.
+* `[out] result`: Whether the feature or a set of features are enabled.
+
+Returns `napi_ok` if the API succeeded.
+
+The function checks enabled features for the module.
+If `feature` parameter has multiple `napi_features` bit flags, then the
+function returns `true` only when all the requested fatures are enabled.
+
+See [`napi_features`][] for more details about Node-API features.
+
 ## Memory management
 
 ### `napi_adjust_external_memory`

diff --git a/src/js_native_api.h b/src/js_native_api.h
@@ -566,6 +566,11 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_object_seal(napi_env env,
                                                     napi_value object);
 #endif  // NAPI_VERSION >= 8
 
+#ifdef NAPI_EXPERIMENTAL
+NAPI_EXTERN napi_status NAPI_CDECL
+napi_is_feature_enabled(napi_env env, napi_features feature, bool* result);
+#endif  // NAPI_EXPERIMENTAL
+
 EXTERN_C_END
 
 #endif  // SRC_JS_NATIVE_API_H_
diff --git a/src/js_native_api_types.h b/src/js_native_api_types.h
@@ -108,6 +108,37 @@ typedef enum {
 //   * the definition of `napi_status` in doc/api/n-api.md to reflect the newly
 //     added value(s).
 
+#ifdef NAPI_EXPERIMENTAL
+// Features allow changing internal behavior of existing Node-API functions.
+//
+// We pass a napi_features pointer to the napi_module struct
+// in the NAPI_MODULE_X macro. This macro is used for the module registration.
+// If the module is initialized without using this macro, then there will be
+// no features selected and the module will use the napi_feature_none.
+//
+// Each Node-API version defines its own default set of features.
+// For the current version it can be accessed using napi_default_features.
+// A module can override the set of its enabled features by adding
+// NAPI_CUSTOM_FEATURES definition to the .gyp file and then defining the
+// value of the global napi_module_features variable.
+// To check enabled features use the `napi_is_feature_enabled` function.
+//
+// For example, to disables napi_feature_reference_all_types:
+// napi_features napi_module_features =
+//   napi_default_features & ~napi_feature_reference_all_types;
+typedef enum {
+  // To be used when no features needs to be set.
+  napi_feature_none = 0,
+  // Use napi_ref for all value types.
+  // Not only objects, functions, and symbols as before.
+  napi_feature_reference_all_types = 1 << 0,
+  // Each version of NAPI is going to have its own default set of features.
+  napi_default_experimental_features = napi_feature_reference_all_types,
+  // This variable must be conditionally set depending on the NAPI version.
+  napi_default_features = napi_default_experimental_features,
+} napi_features;
+#endif
+
 typedef napi_value(NAPI_CDECL* napi_callback)(napi_env env,
                                               napi_callback_info info);
 typedef void(NAPI_CDECL* napi_finalize)(napi_env env,

diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc
@@ -576,7 +576,9 @@ Reference::Reference(napi_env env, v8::Local<v8::Value> value, Args&&... args)
     : RefBase(env, std::forward<Args>(args)...),
       _persistent(env->isolate, value),
       _secondPassParameter(new SecondPassCallParameterRef(this)),
-      _secondPassScheduled(false) {
+      _secondPassScheduled(false),
+      _canBeWeak(!env->IsFeatureEnabled(napi_feature_reference_all_types) ||
-      _canBeWeak(!env->IsFeatureEnabled(napi_feature_reference_all_types) ||
+      _canBeWeak(
-      _canBeWeak(!env->IsFeatureEnabled(napi_feature_reference_all_types) ||
+      _canBeWeak(
+                 value->IsObject() || value->IsFunction()) {
   if (RefCount() == 0) {
     SetWeak();
   }
@@ -652,7 +654,7 @@ void Reference::Finalize(bool is_env_teardown) {
 // the secondPassParameter so that even if it has been
 // scheduled no Finalization will be run.
 void Reference::ClearWeak() {
-  if (!_persistent.IsEmpty()) {
+  if (!_persistent.IsEmpty() && _canBeWeak) {
     _persistent.ClearWeak();
   }
   if (_secondPassParameter != nullptr) {
@@ -669,8 +671,13 @@ void Reference::SetWeak() {
     // nothing
     return;
   }
-  _persistent.SetWeak(
-      _secondPassParameter, FinalizeCallback, v8::WeakCallbackType::kParameter);
+  if (_canBeWeak) {
+    _persistent.SetWeak(_secondPassParameter,
+                        FinalizeCallback,
+                        v8::WeakCallbackType::kParameter);
+  } else {
+    _persistent.Reset();
+  }
   *_secondPassParameter = this;
 }
 
@@ -2495,9 +2502,11 @@ napi_status NAPI_CDECL napi_create_reference(napi_env env,
   CHECK_ARG(env, result);
 
   v8::Local<v8::Value> v8_value = v8impl::V8LocalValueFromJsValue(value);
-  if (!(v8_value->IsObject() || v8_value->IsFunction() ||
-        v8_value->IsSymbol())) {
-    return napi_set_last_error(env, napi_invalid_arg);
+  if (!env->IsFeatureEnabled(napi_feature_reference_all_types)) {
+    if (!(v8_value->IsObject() || v8_value->IsFunction() ||
+          v8_value->IsSymbol())) {
+      return napi_set_last_error(env, napi_invalid_arg);
+    }
   }
 
   v8impl::Reference* reference =
@@ -3257,3 +3266,12 @@ napi_status NAPI_CDECL napi_is_detached_arraybuffer(napi_env env,
 
   return napi_clear_last_error(env);
 }
+
+napi_status NAPI_CDECL napi_is_feature_enabled(napi_env env,
+                                               napi_features feature,
+                                               bool* result) {
+  CHECK_ENV(env);
+  CHECK_ARG(env, result);
+  *result = env->IsFeatureEnabled(feature);
+  return napi_clear_last_error(env);
+}
diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h
@@ -50,9 +50,10 @@ class RefTracker {
 }  // end of namespace v8impl
 
 struct napi_env__ {
-  explicit napi_env__(v8::Local<v8::Context> context)
+  explicit napi_env__(v8::Local<v8::Context> context, napi_features* features)
       : isolate(context->GetIsolate()), context_persistent(isolate, context) {
     CHECK_EQ(isolate, context->GetIsolate());
+    SetFeatures(features);
     napi_clear_last_error(this);
   }
 
@@ -89,13 +90,28 @@ struct napi_env__ {
     }
   }
 
-  // This should be overridden to schedule the finalization to a properiate
+  // This should be overridden to schedule the finalization to appropriate
   // timing, like next tick of the event loop.
   virtual void CallFinalizer(napi_finalize cb, void* data, void* hint) {
     v8::HandleScope handle_scope(isolate);
     CallIntoModule([&](napi_env env) { cb(env, data, hint); });
   }
 
+  bool IsFeatureEnabled(napi_features feature) {
+    // By comparing results of `&` operation to the feature parameter
+    // we allow to test for multiple feature flags.
+    return (_features & feature) == feature;
+  }
+
+  void SetFeatures(napi_features* features) {
+    if (features == nullptr) {
+      _features = napi_feature_none;
+    } else {
+      const napi_features availableFeatures = napi_default_features;
+      _features = static_cast<napi_features>(availableFeatures & *features);
+    }
+  }
+
   virtual void DeleteMe() {
     // First we must finalize those references that have `napi_finalizer`
     // callbacks. The reason is that addons might store other references which
@@ -122,6 +138,7 @@ struct napi_env__ {
   int open_callback_scopes = 0;
   int refs = 1;
   void* instance_data = nullptr;
+  napi_features _features = napi_feature_none;
 
  protected:
   // Should not be deleted directly. Delete with `napi_env__::DeleteMe()`
@@ -435,6 +452,7 @@ class Reference : public RefBase {
   v8impl::Persistent<v8::Value> _persistent;
   SecondPassCallParameterRef* _secondPassParameter;
   bool _secondPassScheduled;
+  const bool _canBeWeak;
 
   FRIEND_TEST(JsNativeApiV8Test, Reference);
 };

diff --git a/src/node_api.cc b/src/node_api.cc
@@ -20,8 +20,9 @@
 #include <memory>
 
 node_napi_env__::node_napi_env__(v8::Local<v8::Context> context,
-                                 const std::string& module_filename)
-    : napi_env__(context), filename(module_filename) {
+                                 const std::string& module_filename,
+                                 napi_features* features)
+    : napi_env__(context, features), filename(module_filename) {
   CHECK_NOT_NULL(node_env());
 }
 
@@ -126,10 +127,11 @@ class BufferFinalizer : private Finalizer {
 };
 
 inline napi_env NewEnv(v8::Local<v8::Context> context,
-                       const std::string& module_filename) {
+                       const std::string& module_filename,
+                       napi_features* features) {
   node_napi_env result;
 
-  result = new node_napi_env__(context, module_filename);
+  result = new node_napi_env__(context, module_filename, features);
   // TODO(addaleax): There was previously code that tried to delete the
   // napi_env when its v8::Context was garbage collected;
   // However, as long as N-API addons using this napi_env are in place,
@@ -586,24 +588,42 @@ class AsyncContext {
 
 }  // end of namespace v8impl
 
+void napi_module_register_by_symbol_with_features(
+    v8::Local<v8::Object> exports,
+    v8::Local<v8::Value> module,
+    v8::Local<v8::Context> context,
+    napi_addon_register_func init,
+    napi_features* features);
+
 // Intercepts the Node-V8 module registration callback. Converts parameters
 // to NAPI equivalents and then calls the registration callback specified
 // by the NAPI module.
 static void napi_module_register_cb(v8::Local<v8::Object> exports,
                                     v8::Local<v8::Value> module,
                                     v8::Local<v8::Context> context,
                                     void* priv) {
-  napi_module_register_by_symbol(
+  napi_module_register_by_symbol_with_features(
       exports,
       module,
       context,
-      static_cast<const napi_module*>(priv)->nm_register_func);
+      static_cast<const napi_module*>(priv)->nm_register_func,
+      static_cast<const napi_module*>(priv)->nm_features);
 }
 
 void napi_module_register_by_symbol(v8::Local<v8::Object> exports,
                                     v8::Local<v8::Value> module,
                                     v8::Local<v8::Context> context,
                                     napi_addon_register_func init) {
+  napi_module_register_by_symbol_with_features(
+      exports, module, context, init, nullptr);
+}
+
+void napi_module_register_by_symbol_with_features(
+    v8::Local<v8::Object> exports,
+    v8::Local<v8::Value> module,
+    v8::Local<v8::Context> context,
+    napi_addon_register_func init,
+    napi_features* features) {
   node::Environment* node_env = node::Environment::GetCurrent(context);
   std::string module_filename = "";
   if (init == nullptr) {
@@ -631,7 +651,7 @@ void napi_module_register_by_symbol(v8::Local<v8::Object> exports,
   }
 
   // Create a new napi_env for this specific module.
-  napi_env env = v8impl::NewEnv(context, module_filename);
+  napi_env env = v8impl::NewEnv(context, module_filename, features);
 
   napi_value _exports;
   env->CallIntoModule([&](napi_env env) {

diff --git a/src/node_api.h b/src/node_api.h
@@ -38,7 +38,12 @@ typedef struct napi_module {
   napi_addon_register_func nm_register_func;
   const char* nm_modname;
   void* nm_priv;
+#ifdef NAPI_EXPERIMENTAL
+  napi_features* nm_features;
+  void* reserved[3];
+#else
   void* reserved[4];
+#endif
 } napi_module;
 
 #define NAPI_MODULE_VERSION 1
@@ -73,18 +78,43 @@ typedef struct napi_module {
   static void fn(void)
 #endif
 
+#ifdef NAPI_EXPERIMENTAL
+#ifdef NAPI_CUSTOM_FEATURES
+
+// Define value of napi_module_features variable in your module when
+// NAPI_CUSTOM_FEATURES is set in gyp file.
+extern napi_features napi_module_features;
+#define NAPI_DEFINE_DEFAULT_FEATURES
+
+#else  // NAPI_CUSTOM_FEATURES
+
+#define NAPI_DEFINE_DEFAULT_FEATURES                                           \
+  static napi_features napi_module_features = napi_default_features;
+
+#endif  // NAPI_CUSTOM_FEATURES
+
+#define NAPI_FEATURES_PTR /* NOLINT */ &napi_module_features,
+
+#else  // NAPI_EXPERIMENTAL
+#define NAPI_DEFINE_DEFAULT_FEATURES
+#define NAPI_FEATURES_PTR
+#endif  // NAPI_EXPERIMENTAL
+
 #define NAPI_MODULE_X(modname, regfunc, priv, flags)                           \
   EXTERN_C_START                                                               \
+  NAPI_DEFINE_DEFAULT_FEATURES                                                 \
   static napi_module _module = {                                               \
       NAPI_MODULE_VERSION,                                                     \
       flags,                                                                   \
       __FILE__,                                                                \
       regfunc,                                                                 \
       #modname,                                                                \
       priv,                                                                    \
-      {0},                                                                     \
+      NAPI_FEATURES_PTR{0},                                                    \
   };                                                                           \
-  NAPI_C_CTOR(_register_##modname) { napi_module_register(&_module); }         \
+  NAPI_C_CTOR(_register_##modname) {                                           \
+    napi_module_register(&_module);                                            \
+  }                                                                            \
   EXTERN_C_END
 
 #define NAPI_MODULE_INITIALIZER_X(base, version)                               \