From 3078ccf22ca45b118a7bb138556b06ecb31d3099 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Fri, 6 Jul 2018 10:47:48 -0400 Subject: [PATCH] n-api: create functions directly Avoid using `v8::FunctionTemplate::New()` when using `v8::Function::New()` suffices. This ensures that individual functions can be gc-ed and that functions can be created dynamically without running out of memory. PR-URL: https://github.com/nodejs/node/pull/21688 Reviewed-By: Anna Henningsen Reviewed-By: Kyle Farnung Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Yang Guo Reviewed-By: Colin Ihrig Reviewed-By: Gus Caplan --- src/node_api.cc | 20 +++-- test/addons-napi/test_function/test.js | 8 +- .../addons-napi/test_function/test_function.c | 86 ++++++++++++++++++- 3 files changed, 104 insertions(+), 10 deletions(-) diff --git a/src/node_api.cc b/src/node_api.cc index 3b47b647275065..b2bf6b603a8dbe 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -1023,11 +1023,11 @@ napi_status napi_create_function(napi_env env, RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure); - v8::Local tpl = v8::FunctionTemplate::New( - isolate, v8impl::FunctionCallbackWrapper::Invoke, cbdata); - v8::Local context = isolate->GetCurrentContext(); - v8::MaybeLocal maybe_function = tpl->GetFunction(context); + v8::MaybeLocal maybe_function = + v8::Function::New(context, + v8impl::FunctionCallbackWrapper::Invoke, + cbdata); CHECK_MAYBE_EMPTY(env, maybe_function, napi_generic_failure); return_value = scope.Escape(maybe_function.ToLocalChecked()); @@ -1489,13 +1489,17 @@ napi_status napi_define_properties(napi_env env, v8::Local cbdata = v8impl::CreateFunctionCallbackData(env, p->method, p->data); - RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure); + CHECK_MAYBE_EMPTY(env, cbdata, napi_generic_failure); + + v8::MaybeLocal maybe_fn = + v8::Function::New(context, + v8impl::FunctionCallbackWrapper::Invoke, + cbdata); - v8::Local t = v8::FunctionTemplate::New( - isolate, v8impl::FunctionCallbackWrapper::Invoke, cbdata); + CHECK_MAYBE_EMPTY(env, maybe_fn, napi_generic_failure); auto define_maybe = obj->DefineOwnProperty( - context, property_name, t->GetFunction(), attributes); + context, property_name, maybe_fn.ToLocalChecked(), attributes); if (!define_maybe.FromMaybe(false)) { return napi_set_last_error(env, napi_generic_failure); diff --git a/test/addons-napi/test_function/test.js b/test/addons-napi/test_function/test.js index 752e9965b23039..6f0f1681752a2c 100644 --- a/test/addons-napi/test_function/test.js +++ b/test/addons-napi/test_function/test.js @@ -1,11 +1,12 @@ 'use strict'; +// Flags: --expose-gc + const common = require('../../common'); const assert = require('assert'); // testing api calls for function const test_function = require(`./build/${common.buildType}/test_function`); - function func1() { return 1; } @@ -29,3 +30,8 @@ assert.strictEqual(test_function.TestCall(func4, 1), 2); assert.strictEqual(test_function.TestName.name, 'Name'); assert.strictEqual(test_function.TestNameShort.name, 'Name_'); + +let tracked_function = test_function.MakeTrackedFunction(common.mustCall()); +assert(!!tracked_function); +tracked_function = null; +global.gc(); diff --git a/test/addons-napi/test_function/test_function.c b/test/addons-napi/test_function/test_function.c index b0350e6ee22876..149b7e3a13a806 100644 --- a/test/addons-napi/test_function/test_function.c +++ b/test/addons-napi/test_function/test_function.c @@ -30,7 +30,79 @@ napi_value TestFunctionName(napi_env env, napi_callback_info info) { return NULL; } -napi_value Init(napi_env env, napi_value exports) { +static void finalize_function(napi_env env, void* data, void* hint) { + napi_ref ref = data; + + // Retrieve the JavaScript undefined value. + napi_value undefined; + NAPI_CALL_RETURN_VOID(env, napi_get_undefined(env, &undefined)); + + // Retrieve the JavaScript function we must call. + napi_value js_function; + NAPI_CALL_RETURN_VOID(env, napi_get_reference_value(env, ref, &js_function)); + + // Call the JavaScript function to indicate that the generated JavaScript + // function is about to be gc-ed. + NAPI_CALL_RETURN_VOID(env, napi_call_function(env, + undefined, + js_function, + 0, + NULL, + NULL)); + + // Destroy the persistent reference to the function we just called so as to + // properly clean up. + NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, ref)); +} + +static napi_value MakeTrackedFunction(napi_env env, napi_callback_info info) { + size_t argc = 1; + napi_value js_finalize_cb; + napi_valuetype arg_type; + + // Retrieve and validate from the arguments the function we will use to + // indicate to JavaScript that the function we are about to create is about to + // be gc-ed. + NAPI_CALL(env, napi_get_cb_info(env, + info, + &argc, + &js_finalize_cb, + NULL, + NULL)); + NAPI_ASSERT(env, argc == 1, "Wrong number of arguments"); + NAPI_CALL(env, napi_typeof(env, js_finalize_cb, &arg_type)); + NAPI_ASSERT(env, arg_type == napi_function, "Argument must be a function"); + + // Dynamically create a function. + napi_value result; + NAPI_CALL(env, napi_create_function(env, + "TrackedFunction", + NAPI_AUTO_LENGTH, + TestFunctionName, + NULL, + &result)); + + // Create a strong reference to the function we will call when the tracked + // function is about to be gc-ed. + napi_ref js_finalize_cb_ref; + NAPI_CALL(env, napi_create_reference(env, + js_finalize_cb, + 1, + &js_finalize_cb_ref)); + + // Attach a finalizer to the dynamically created function and pass it the + // strong reference we created in the previous step. + NAPI_CALL(env, napi_wrap(env, + result, + js_finalize_cb_ref, + finalize_function, + NULL, + NULL)); + + return result; +} + +static napi_value Init(napi_env env, napi_value exports) { napi_value fn1; NAPI_CALL(env, napi_create_function( env, NULL, NAPI_AUTO_LENGTH, TestCallFunction, NULL, &fn1)); @@ -43,9 +115,21 @@ napi_value Init(napi_env env, napi_value exports) { NAPI_CALL(env, napi_create_function( env, "Name_extra", 5, TestFunctionName, NULL, &fn3)); + napi_value fn4; + NAPI_CALL(env, napi_create_function(env, + "MakeTrackedFunction", + NAPI_AUTO_LENGTH, + MakeTrackedFunction, + NULL, + &fn4)); + NAPI_CALL(env, napi_set_named_property(env, exports, "TestCall", fn1)); NAPI_CALL(env, napi_set_named_property(env, exports, "TestName", fn2)); NAPI_CALL(env, napi_set_named_property(env, exports, "TestNameShort", fn3)); + NAPI_CALL(env, napi_set_named_property(env, + exports, + "MakeTrackedFunction", + fn4)); return exports; }