Finalizer for napi_create_function()

[gabrielschulhof]

When using napi_create_object() and napi_create_function(), you cannot specify a finalizer callback, which probably should be allowed since it can be useful to have.

The omission of a napi_finalize for napi_create_function() is of particular concern IMO.

Re: https://github.com/nodejs/node/issues/14256

[gabrielschulhof]

... because it prevents one from being able to associate dynamic data with a function.

OTOH, how often to we dynamically associate a native function with a JS function? Most often we do so from Init() and then never again.

[gabrielschulhof]

@nodejs/n-api I think the reason we don't have napi_finalize on napi_create_function() and napi_define_class() is that native-backed JS functions live forever – at least on V8. Similarly, IIRC, the v8::FunctionTemplate also lives forever.

IMO this also means we should discourage function factories. Of course, we still may want to add finalizers for the sake of other engines.

[gabrielschulhof]

If we were to implement this, we couldn't really test it on V8.

[mhdawson]

Unless the team from ChakraCore thinks this is important, I'd probably focus on other priorities as opposed to adding a new method with the additional signature.

[gabrielschulhof]

@kfarnung @digitalinfinity what do you think?

[digitalinfinity]

I agree with the sentiment that it could be useful, but I'm also of the view that we should generally wait for module authors to say that something is impossible for them to implement in N-API without a new API. Or in other words, I'm in favor of kicking this can down the road until someone convinces us otherwise.

[gabrielschulhof]

If we sacrifice the ability to wrap a function, we can already accomplish this:

void delete_data(napi_env env, void* data, void* hint) {
  free(data);
}

NativeData* data = malloc(*data);
napi_value js_function;

assert(napi_create_function(env,
                            "someMethod",
                            NAPI_AUTO_LENGTH,
                            data,
                            &js_function) == napi_ok);
assert(napi_wrap(env, js_function, data, delete_data, NULL, NULL) == napi_ok);

[gabrielschulhof]

In the API working group meeting of 2018-08-09 we agreed to add an API for adding finalizers so that data associated with dynamically created functions can itself be freed when the function is finalized while at the same time advising potential bug reporters to rely on napi_wrap() until the new API becomes supported on all supported versions of Node.js.

Relying on napi_wrap(), although not ideal, should not be a big sacrifice because, although it sacrifices the ability to wrap a function, it is usually not functions that are wrapped, but rather objects which are either plain or constructed using functions.