idea: detect JS calls at compile time

[gabrielschulhof]

Recording this idea so it doesn't slip under the radar:

In support of pure finalizers, I'm wondering if it's possible to templetize napi::Environment such that one specialization attaches js-execution-capable finalizers whereas the other attaches pure finalizers. We can use type inference to specialize the right class for a given user-provided finalizer.

That is, if the user's finalizer, after inlining and all, contains no calls to functions that execute JS, then we specialize the environment that attaches pure finalizers, otherwise we specialize the environment that does not. That way, add-on maintainers whose finalizers do not really execute javascript could simply recompile and have their memory leak worries go away without any code changes, once https://github.com/nodejs/node/pull/42651 lands.

The reason I chose napi::Environment as the "probe" which will tell us whether the user's finalizer ends up calling a Node-API that executes JS is that all Node-APIs require an environment, so the type inference will snap to a non-pure napi::Environment if there is even one call to a JS-executing Node-API in the user's code.

We can actually work on this before https://github.com/nodejs/node/pull/42651 lands, but for now we'll attach finalizers the same way whether or not their body calls into JS. Maybe we can use #warning to output compiler messages indicating whether the finalizer would have been pure or not.

Another aspect is this: if there are two napi::Environment classes available and you pass the wrong one to the API that sets up the finalizer, it will fail to compile if you pass a "pure" environment along with a finalizer that does not accept such a "pure" environment.

[gabrielschulhof]

After discussing at today's Node-API meeting, we concluded that, in core, we should guard the new pure env functionality with an additional NODE_API_EXPERIMENTAL_PURE_ENV flag. The motivation and policy are as follow:

• An additional flag the name of which contains the feature allows us to have multiple concurrent feature-specific flags within NAPI_EXPERIMENTAL, allowing adopters running with NAPI_EXPERIMENTAL to independently adopt each experimental feature.
• The flags will be removed along with NAPI_EXPERIMENTAL when we make a new Node-API release.
• The motivation for these flags is that we are breaking APIs we have already released and which are already stable, even though we're breaking them without any corresponding ABI breakage. This would cause the type of breakage beyond what we warn about in conjunction with the NAPI_EXPERIMENTAL flag.

[gabrielschulhof]

https://github.com/nodejs/node/pull/50060 is the first step. Then we have to turn on NODE_API_EXPERIMENTAL_PURE_ENV in node-addon-api and plumb the changes through its code base.

[github-actions[bot]]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

[github-actions[bot]]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.