node-api: segregate pure and non-pure APIs via type system

We define a new type called `node_api_pure_env` as the `const` version
of `napi_env` and `node_api_pure_finalize` as a variant of
`napi_finalize` that accepts a `node_api_pure_env` as its first
argument.

We then modify those APIs which do not affect GC state as accepting a
`node_api_pure_env`. APIs accepting finalizer callbacks are modified to
accept `node_api_pure_finalize` callbacks. Thus, the only way to attach
a `napi_finalize` callback, wherein Node-APIs affecting GC state may be
called is to call `node_api_post_finalizer` from a
`node_api_pure_finalize` callback.

In keeping with the process of introducing new Node-APIs, this feature
is guarded by `NAPI_EXPERIMENTAL`. Additionally, and since this feature
modifies APIs already marked as stable, it is additionally guared by
`NODE_API_EXPERIMENTAL_PURE_ENV`, so as to provide a further buffer to
adoption. Nevertheless, both guards must be removed upon releasing a
new version of Node-API.

Author: gabrielschulhof

doc/contributing/adding-new-napi-api.md

@@ -47,3 +47,12 @@ Node-API.
       to the decision to take an API out of experimental status.
     * The API **must** be implemented in a Node.js implementation with an
       alternate VM.
+
+Since the adoption of the policy whereby moving to a later version of Node-API
+from an earlier version may entail rework of existing code, it is possible to
+introduce modifications to already-released Node-APIs, as long as the
+modifications affect neither the ABI nor the API of earlier versions. Such
+modifications **must** be guarded, in addition to the above-mentioned
+compile-time flag, by a further compile-time flag whose name reflects the
+nature of the modification being made to existing APIs. This provides a buffer
+to adoption above and beyond the one provided by the initial compile-time flag.

doc/contributing/releases-node-api.md

@@ -84,7 +84,9 @@ define guards on the declaration of the new Node-API. Check for these guards
 with:
 
 ```bash
-grep NAPI_EXPERIMENTAL src/js_native_api{_types,}.h src/node_api{_types,}.h
+grep \
+  -E \
+  N(ODE_)?API_EXPERIMENTAL src/js_native_api{_types,}.h src/node_api{_types,}.h
 ```
 
 and update the define version guards with the release version:
@@ -100,6 +102,9 @@ and update the define version guards with the release version:
 + #endif  // NAPI_VERSION >= 10
 ```
 
+Remove any additional `NODE_API_EXPERIMENTAL_*` guards along with
+`NAPI_EXPERIMENTAL`.
+
 Also, update the Node-API version value of the `napi_get_version` test in
 `test/js-native-api/test_general/test.js` with the release version `x`:
 
@@ -128,7 +133,7 @@ commits should already include `NAPI_EXPERIMENTAL` definition for the tests.
 Check for these definitions with:
 
 ```bash
-grep NAPI_EXPERIMENTAL test/node-api/*/{*.{h,c},binding.gyp} test/js-native-api/*/{*.{h,c},binding.gyp}
+grep -E N(ODE_)?API_EXPERIMENTAL test/node-api/*/{*.{h,c},binding.gyp} test/js-native-api/*/{*.{h,c},binding.gyp}
 ```
 
 and substitute the `NAPI_EXPERIMENTAL` with the release version
@@ -139,6 +144,8 @@ and substitute the `NAPI_EXPERIMENTAL` with the release version
 + #define NAPI_VERSION 10
 ```
 
+Remove any `NODE_API_EXPERIMENTAL_*` flags.
+
 #### Step 4. Update document
 
 If this release includes new Node-APIs that were first released in this

src/js_native_api.h

@@ -49,8 +49,8 @@
 
 EXTERN_C_START
 
-NAPI_EXTERN napi_status NAPI_CDECL
-napi_get_last_error_info(napi_env env, const napi_extended_error_info** result);
+NAPI_EXTERN napi_status NAPI_CDECL napi_get_last_error_info(
+    node_api_pure_env env, const napi_extended_error_info** result);
 
 // Getters for defined singletons
 NAPI_EXTERN napi_status NAPI_CDECL napi_get_undefined(napi_env env,
@@ -97,15 +97,15 @@ NAPI_EXTERN napi_status NAPI_CDECL
 node_api_create_external_string_latin1(napi_env env,
                                        char* str,
                                        size_t length,
-                                       napi_finalize finalize_callback,
+                                       node_api_pure_finalize finalize_callback,
                                        void* finalize_hint,
                                        napi_value* result,
                                        bool* copied);
 NAPI_EXTERN napi_status NAPI_CDECL
 node_api_create_external_string_utf16(napi_env env,
                                       char16_t* str,
                                       size_t length,
-                                      napi_finalize finalize_callback,
+                                      node_api_pure_finalize finalize_callback,
                                       void* finalize_hint,
                                       napi_value* result,
                                       bool* copied);
@@ -289,7 +289,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_instanceof(napi_env env,
 
 // Gets all callback info in a single call. (Ugly, but faster.)
 NAPI_EXTERN napi_status NAPI_CDECL napi_get_cb_info(
-    napi_env env,               // [in] NAPI environment handle
+    node_api_pure_env env,      // [in] NAPI environment handle
     napi_callback_info cbinfo,  // [in] Opaque callback-info handle
     size_t* argc,      // [in-out] Specifies the size of the provided argv array
                        // and receives the actual count of args.
@@ -313,7 +313,7 @@ napi_define_class(napi_env env,
 NAPI_EXTERN napi_status NAPI_CDECL napi_wrap(napi_env env,
                                              napi_value js_object,
                                              void* native_object,
-                                             napi_finalize finalize_cb,
+                                             node_api_pure_finalize finalize_cb,
                                              void* finalize_hint,
                                              napi_ref* result);
 NAPI_EXTERN napi_status NAPI_CDECL napi_unwrap(napi_env env,
@@ -325,7 +325,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_remove_wrap(napi_env env,
 NAPI_EXTERN napi_status NAPI_CDECL
 napi_create_external(napi_env env,
                      void* data,
-                     napi_finalize finalize_cb,
+                     node_api_pure_finalize finalize_cb,
                      void* finalize_hint,
                      napi_value* result);
 NAPI_EXTERN napi_status NAPI_CDECL napi_get_value_external(napi_env env,
@@ -424,7 +424,7 @@ NAPI_EXTERN napi_status NAPI_CDECL
 napi_create_external_arraybuffer(napi_env env,
                                  void* external_data,
                                  size_t byte_length,
-                                 napi_finalize finalize_cb,
+                                 node_api_pure_finalize finalize_cb,
                                  void* finalize_hint,
                                  napi_value* result);
 #endif  // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
@@ -466,7 +466,7 @@ napi_get_dataview_info(napi_env env,
                        size_t* byte_offset);
 
 // version management
-NAPI_EXTERN napi_status NAPI_CDECL napi_get_version(napi_env env,
+NAPI_EXTERN napi_status NAPI_CDECL napi_get_version(node_api_pure_env env,
                                                     uint32_t* result);
 
 // Promises
@@ -490,7 +490,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_run_script(napi_env env,
 
 // Memory management
 NAPI_EXTERN napi_status NAPI_CDECL napi_adjust_external_memory(
-    napi_env env, int64_t change_in_bytes, int64_t* adjusted_value);
+    node_api_pure_env env, int64_t change_in_bytes, int64_t* adjusted_value);
 
 #if NAPI_VERSION >= 5
 
@@ -508,19 +508,20 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_get_date_value(napi_env env,
                                                        double* result);
 
 // Add finalizer for pointer
-NAPI_EXTERN napi_status NAPI_CDECL napi_add_finalizer(napi_env env,
-                                                      napi_value js_object,
-                                                      void* finalize_data,
-                                                      napi_finalize finalize_cb,
-                                                      void* finalize_hint,
-                                                      napi_ref* result);
+NAPI_EXTERN napi_status NAPI_CDECL
+napi_add_finalizer(napi_env env,
+                   napi_value js_object,
+                   void* finalize_data,
+                   node_api_pure_finalize finalize_cb,
+                   void* finalize_hint,
+                   napi_ref* result);
 
 #endif  // NAPI_VERSION >= 5
 
 #ifdef NAPI_EXPERIMENTAL
 
 NAPI_EXTERN napi_status NAPI_CDECL
-node_api_post_finalizer(napi_env env,
+node_api_post_finalizer(node_api_pure_env env,
                         napi_finalize finalize_cb,
                         void* finalize_data,
                         void* finalize_hint);
@@ -564,10 +565,13 @@ napi_get_all_property_names(napi_env env,
                             napi_value* result);
 
 // Instance data
-NAPI_EXTERN napi_status NAPI_CDECL napi_set_instance_data(
-    napi_env env, void* data, napi_finalize finalize_cb, void* finalize_hint);
+NAPI_EXTERN napi_status NAPI_CDECL
+napi_set_instance_data(node_api_pure_env env,
+                       void* data,
+                       node_api_pure_finalize finalize_cb,
+                       void* finalize_hint);
 
-NAPI_EXTERN napi_status NAPI_CDECL napi_get_instance_data(napi_env env,
+NAPI_EXTERN napi_status NAPI_CDECL napi_get_instance_data(node_api_pure_env env,
                                                           void** data);
 #endif  // NAPI_VERSION >= 6
 

src/js_native_api_types.h

@@ -22,6 +22,32 @@ typedef uint16_t char16_t;
 // JSVM API types are all opaque pointers for ABI stability
 // typedef undefined structs instead of void* for compile time type safety
 typedef struct napi_env__* napi_env;
+
+// We need to mark APIs which are "pure", meaning that they do not interact
+// with the JS engine, and can therefore be called synchronously from a
+// finalizer that itself runs synchronously during garbage collection. Such
+// "pure" APIs can receive either a `napi_env` or a `node_api_pure_env` as
+// their first parameter, because we should be able to also call them during
+// normal, non-garbage-collecting operations, whereas "non-pure" APIs can only
+// receive a `napi_env` as their first parameter, because we must not call them
+// during garbage collection. In lieu of inheritance, we use the properties of
+// the const qualifier to accomplish this, because both a const and a non-const
+// value can be passed to an API expecting a const value, but only a non-const
+// value can be passed to an API expecting a non-const value.
+//
+// In conjunction with appropriate CFLAGS to warn us if we're passing a const
+// (pure) environment into an API that expects a non-const (non-pure)
+// environment, and the definition of pure finalizer function pointer types
+// below, which receive a pure environment as their first parameter, and can
+// thus only call pure APIs (unless the user explicitly casts the environment),
+// we achieve the ability to ensure at compile time that we do not call non-
+// pure APIs from a synchronous (pure) finalizer.
+#if defined(NAPI_EXPERIMENTAL) && defined(NODE_API_EXPERIMENTAL_PURE_ENV)
+typedef const struct napi_env__* node_api_pure_env;
+#else
+typedef struct napi_env__* node_api_pure_env;
+#endif  // NAPI_EXPERIMENTAL && NODE_API_EXPERIMENTAL_PURE_ENV
+
 typedef struct napi_value__* napi_value;
 typedef struct napi_ref__* napi_ref;
 typedef struct napi_handle_scope__* napi_handle_scope;
@@ -115,6 +141,13 @@ typedef napi_value(NAPI_CDECL* napi_callback)(napi_env env,
 typedef void(NAPI_CDECL* napi_finalize)(napi_env env,
                                         void* finalize_data,
                                         void* finalize_hint);
+#if defined(NAPI_EXPERIMENTAL) && defined(NODE_API_EXPERIMENTAL_PURE_ENV)
+typedef void(NAPI_CDECL* node_api_pure_finalize)(node_api_pure_env env,
+                                                 void* finalize_data,
+                                                 void* finalize_hint);
+#else
+typedef napi_finalize node_api_pure_finalize;
+#endif  // NAPI_EXPERIMENTAL && NODE_API_EXPERIMENTAL_PURE_ENV
 
 typedef struct {
   // One of utf8name or name should be NULL.