node-api: get Node API version used by addon

[vmoroz]

This is to address the issue #45657.

The main goal of the PR is to establish a new mechanism to implement Node-API changes that affect not only the API shape, but also its behavior.
We are going to associate Node-API functions and new Node-API behavior with new Node-API version.
Node-API modules can choose which version of Node-API they target.
By default, they target the current Node-API version 8.
To target newer versions of Node-API they must explicitly opt-in by defining napi_module_get_api_version_v1 export function that returns targeted version.

In this PR, the NAPI_MODULE and NAPI_MODULE_INITIALIZER macros are changed to define the napi_module_get_api_version_v1 function that reports the value of the NAPI_VERSION macro. C++ developers can control the targeted Node-API version by defining the NAPI_VERSION macro as they did before. The key difference is that now this macro not only affects the set of functions that module uses but also affects the Node-API implementation behavior.

Node.js is going to validate the version requested by the addon.

• If the version is below version 8, then it will be internally switched to version 8 because we did not implement the behavior change before version 8.
• If the version is above the NAPI_VERSION the Node.js compiled for and it is not NAPI_VERSION_EXPERIMENTAL, then Node.js will throw an error on module initialization.

Note that modules targeting NAPI_VERSION_EXPERIMENTAL are compatible with any Node-API version the Node.js was compiled with. Node-API in that case will use the latest functions and behaviors. Such module should call napi_get_version function to get the latest stable Node-API version offered by current Node.js instance. Then it must adapt for the different behavior based on the version.

In this PR, we change the behavior of the napi_create_reference function for Node-API versions higher than 8. The new behavior allows the creation of an napi_ref for any napi_valuetype instead of only napi_object, napi_function, napi_external, and napi_symbol. One of the main goals is to enable the use of napi_ref for napi_string.

The napi_ref for the new primitive types immediately releases them when the ref count becomes 0 because they cannot be weak references. For the previously supported object and symbol types, the behavior did not change: these values become weak references and can be collected at any time. Note that the globally registered symbols created with the Symbol.for() function call and well-known symbols such as Symbol.iterator are always kept alive because they are never collected by the garbage collector.

[nodejs-github-bot]

Review requested:

• @nodejs/node-api

[KevinEady]

We discussed this draft's approach in the 9 Dec 2022 Node-API meeting. One question that was brought up is discussion about using two functions for the register-by-symbol method (one for getting version and one for initializing the module) and if this is a good approach versus just using a single method. It was determined that using two methods would be more beneficial because we need to know the targeted runtime version before the module initialization code runs, since this code can run arbitrary JavaScript.

[legendecas]

Thank you for working on this! I know this is still a draft but I'll continue reviewing.

[gabrielschulhof]

Mostly LGTM. Can we combine the two tests into a single test that uses two addons? So, like,

{
  "targets": [
    {
      "target_name": "test_reference_all_types",
      "sources": [ "test_reference_by_node_api_version.c" ],
      "defines": [ "NAPI_EXPERIMENTAL" ],
    },
    {
      "target_name": "test_reference_obj_only",
      "sources": [ "test_reference_by_node_api_version.c" ],
      "defines": [ "NAPI_VERSION=8" ],
    }
  ]
}

... and then do some #ifdef-ing in the source file? The test JS can then require() each add-on and perform a fixed set of tests with two different sets of expectations as passed-in parameters.

[vmoroz]

Mostly LGTM. Can we combine the two tests into a single test that uses two addons? So, like,

Let me see if it can be done.

[gabrielschulhof]

Mostly LGTM. Can we combine the two tests into a single test that uses two addons? So, like,

Let me see if it can be done.

It may also be possible to distill common bits into one source file which is listed in the sources section of both targets and then have two source files, each of which has the version-specific changes, one for each target. That should reduce the need for #ifdefs.

[vmoroz]

Mostly LGTM. Can we combine the two tests into a single test that uses two addons? So, like,

Let me see if it can be done.

It may also be possible to distill common bits into one source file which is listed in the sources section of both targets and then have two source files, each of which has the version-specific changes, one for each target. That should reduce the need for #ifdefs.

I combined both tests and found that the C code between them was exactly the same. So, there was no need for #ifdefs. I also noticed that the JS code was mostly the same and I combined them too with minimal changes. Now the test looks pretty cool because it shows how we can create two different addons from the same C code that have different behavior depending on the targeted Node-API version. Then we use these addons in the same JS code.

[vmoroz]

@legendecas , @mhdawson , the PR is ready to be merged. Could you please take a look at it?

[gabrielschulhof]

I got partway through a review. Will look further.

[mhdawson]

This does match current behaviour but also means that we'll have a leak right? Since we can change behaviour in a new Node-API version without affecting existing applications maybe the right thing to do is to consider all symbols as not being week so that the leak does not occur?

[vmoroz]

@mhdawson, there are no memory leaks. The only side effect is that for the global and well-known symbols we always return a valid symbol because they are global and never collected by GC (it is by design). The local symbols are behaving the same way as weak references to objects.

We say that when the napi_ref ref count goes to zero we are converting values to weak references if they are supported, otherwise we release them. Symbols support weak references, but they do it partially: for some of them the weak reference will always return "live" value because GC never collects them.

There is also a difference in the JavaScript WeakRef behavior. It throws an exception if a global symbol is given to it but succeeds for local and well-known symbols. We can align with this behavior in future when V8 API lets us to see the symbol type. Though we should probably avoid throwing an exception and thus resetting the global symbols instead of keeping them could be not that big of a deal.

In any case to avoid native memory leaks we require to call napi_delete_reference. Changing the ref count was never controlling the memory lifetime as we see it in COM IUnknown or in std::shared_ptr. If the napi_delete_reference is not called explicitly, then the native memory will be removed on napi_env finalization. So, technically it is also not a memory leak, but rather delays the memory release.

[mhdawson]

@vmoroz thanks for the clarification. I was thinking about the auto-delete we have in RefBase but looking more closely I see that won't be related to direct use of references.

[mhdawson]

@vmoroz just one question, otherwise all looks good to me. Will wait to add approval until we agree on the answer to that question.

[mhdawson]

LGTM

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/51631/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/51640/

[legendecas]

Sorry for going in circles about the PR. Thank you for pushing this forward!

[mhdawson]

@vmoroz, thanks for all of the hard on this, landed in c542d3a