Skip to content

Commit 109c599

Browse files
Gabriel Schulhoftargos
Gabriel Schulhof
authored andcommitted
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: #21688 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Kyle Farnung <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Yang Guo <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gus Caplan <[email protected]>
1 parent dc84858 commit 109c599

File tree

3 files changed

+103
-9
lines changed

3 files changed

+103
-9
lines changed

src/node_api.cc

+12-8
Original file line numberDiff line numberDiff line change
@@ -1025,11 +1025,11 @@ napi_status napi_create_function(napi_env env,
10251025

10261026
RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
10271027

1028-
v8::Local<v8::FunctionTemplate> tpl = v8::FunctionTemplate::New(
1029-
isolate, v8impl::FunctionCallbackWrapper::Invoke, cbdata);
1030-
10311028
v8::Local<v8::Context> context = isolate->GetCurrentContext();
1032-
v8::MaybeLocal<v8::Function> maybe_function = tpl->GetFunction(context);
1029+
v8::MaybeLocal<v8::Function> maybe_function =
1030+
v8::Function::New(context,
1031+
v8impl::FunctionCallbackWrapper::Invoke,
1032+
cbdata);
10331033
CHECK_MAYBE_EMPTY(env, maybe_function, napi_generic_failure);
10341034

10351035
return_value = scope.Escape(maybe_function.ToLocalChecked());
@@ -1491,13 +1491,17 @@ napi_status napi_define_properties(napi_env env,
14911491
v8::Local<v8::Value> cbdata =
14921492
v8impl::CreateFunctionCallbackData(env, p->method, p->data);
14931493

1494-
RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
1494+
CHECK_MAYBE_EMPTY(env, cbdata, napi_generic_failure);
1495+
1496+
v8::MaybeLocal<v8::Function> maybe_fn =
1497+
v8::Function::New(context,
1498+
v8impl::FunctionCallbackWrapper::Invoke,
1499+
cbdata);
14951500

1496-
v8::Local<v8::FunctionTemplate> t = v8::FunctionTemplate::New(
1497-
isolate, v8impl::FunctionCallbackWrapper::Invoke, cbdata);
1501+
CHECK_MAYBE_EMPTY(env, maybe_fn, napi_generic_failure);
14981502

14991503
auto define_maybe = obj->DefineOwnProperty(
1500-
context, property_name, t->GetFunction(), attributes);
1504+
context, property_name, maybe_fn.ToLocalChecked(), attributes);
15011505

15021506
if (!define_maybe.FromMaybe(false)) {
15031507
return napi_set_last_error(env, napi_generic_failure);

test/addons-napi/test_function/test.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
'use strict';
2+
// Flags: --expose-gc
3+
24
const common = require('../../common');
35
const assert = require('assert');
46

57
// testing api calls for function
68
const test_function = require(`./build/${common.buildType}/test_function`);
79

8-
910
function func1() {
1011
return 1;
1112
}
@@ -29,3 +30,8 @@ assert.strictEqual(test_function.TestCall(func4, 1), 2);
2930

3031
assert.strictEqual(test_function.TestName.name, 'Name');
3132
assert.strictEqual(test_function.TestNameShort.name, 'Name_');
33+
34+
let tracked_function = test_function.MakeTrackedFunction(common.mustCall());
35+
assert(!!tracked_function);
36+
tracked_function = null;
37+
global.gc();

test/addons-napi/test_function/test_function.c

+84
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,78 @@ static napi_value TestFunctionName(napi_env env, napi_callback_info info) {
3030
return NULL;
3131
}
3232

33+
static void finalize_function(napi_env env, void* data, void* hint) {
34+
napi_ref ref = data;
35+
36+
// Retrieve the JavaScript undefined value.
37+
napi_value undefined;
38+
NAPI_CALL_RETURN_VOID(env, napi_get_undefined(env, &undefined));
39+
40+
// Retrieve the JavaScript function we must call.
41+
napi_value js_function;
42+
NAPI_CALL_RETURN_VOID(env, napi_get_reference_value(env, ref, &js_function));
43+
44+
// Call the JavaScript function to indicate that the generated JavaScript
45+
// function is about to be gc-ed.
46+
NAPI_CALL_RETURN_VOID(env, napi_call_function(env,
47+
undefined,
48+
js_function,
49+
0,
50+
NULL,
51+
NULL));
52+
53+
// Destroy the persistent reference to the function we just called so as to
54+
// properly clean up.
55+
NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, ref));
56+
}
57+
58+
static napi_value MakeTrackedFunction(napi_env env, napi_callback_info info) {
59+
size_t argc = 1;
60+
napi_value js_finalize_cb;
61+
napi_valuetype arg_type;
62+
63+
// Retrieve and validate from the arguments the function we will use to
64+
// indicate to JavaScript that the function we are about to create is about to
65+
// be gc-ed.
66+
NAPI_CALL(env, napi_get_cb_info(env,
67+
info,
68+
&argc,
69+
&js_finalize_cb,
70+
NULL,
71+
NULL));
72+
NAPI_ASSERT(env, argc == 1, "Wrong number of arguments");
73+
NAPI_CALL(env, napi_typeof(env, js_finalize_cb, &arg_type));
74+
NAPI_ASSERT(env, arg_type == napi_function, "Argument must be a function");
75+
76+
// Dynamically create a function.
77+
napi_value result;
78+
NAPI_CALL(env, napi_create_function(env,
79+
"TrackedFunction",
80+
NAPI_AUTO_LENGTH,
81+
TestFunctionName,
82+
NULL,
83+
&result));
84+
85+
// Create a strong reference to the function we will call when the tracked
86+
// function is about to be gc-ed.
87+
napi_ref js_finalize_cb_ref;
88+
NAPI_CALL(env, napi_create_reference(env,
89+
js_finalize_cb,
90+
1,
91+
&js_finalize_cb_ref));
92+
93+
// Attach a finalizer to the dynamically created function and pass it the
94+
// strong reference we created in the previous step.
95+
NAPI_CALL(env, napi_wrap(env,
96+
result,
97+
js_finalize_cb_ref,
98+
finalize_function,
99+
NULL,
100+
NULL));
101+
102+
return result;
103+
}
104+
33105
static napi_value Init(napi_env env, napi_value exports) {
34106
napi_value fn1;
35107
NAPI_CALL(env, napi_create_function(
@@ -43,9 +115,21 @@ static napi_value Init(napi_env env, napi_value exports) {
43115
NAPI_CALL(env, napi_create_function(
44116
env, "Name_extra", 5, TestFunctionName, NULL, &fn3));
45117

118+
napi_value fn4;
119+
NAPI_CALL(env, napi_create_function(env,
120+
"MakeTrackedFunction",
121+
NAPI_AUTO_LENGTH,
122+
MakeTrackedFunction,
123+
NULL,
124+
&fn4));
125+
46126
NAPI_CALL(env, napi_set_named_property(env, exports, "TestCall", fn1));
47127
NAPI_CALL(env, napi_set_named_property(env, exports, "TestName", fn2));
48128
NAPI_CALL(env, napi_set_named_property(env, exports, "TestNameShort", fn3));
129+
NAPI_CALL(env, napi_set_named_property(env,
130+
exports,
131+
"MakeTrackedFunction",
132+
fn4));
49133

50134
return exports;
51135
}

0 commit comments

Comments
 (0)