Skip to content

Commit

Permalink
n-api: create functions directly
Browse files Browse the repository at this point in the history
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 <anna@addaleax.net>
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
  • Loading branch information
Gabriel Schulhof authored and targos committed Jul 10, 2018
1 parent dc84858 commit 109c599
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 9 deletions.
20 changes: 12 additions & 8 deletions src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1025,11 +1025,11 @@ napi_status napi_create_function(napi_env env,

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

v8::Local<v8::FunctionTemplate> tpl = v8::FunctionTemplate::New(
isolate, v8impl::FunctionCallbackWrapper::Invoke, cbdata);

v8::Local<v8::Context> context = isolate->GetCurrentContext();
v8::MaybeLocal<v8::Function> maybe_function = tpl->GetFunction(context);
v8::MaybeLocal<v8::Function> 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());
Expand Down Expand Up @@ -1491,13 +1491,17 @@ napi_status napi_define_properties(napi_env env,
v8::Local<v8::Value> 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<v8::Function> maybe_fn =
v8::Function::New(context,
v8impl::FunctionCallbackWrapper::Invoke,
cbdata);

v8::Local<v8::FunctionTemplate> 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);
Expand Down
8 changes: 7 additions & 1 deletion test/addons-napi/test_function/test.js
Original file line number Diff line number Diff line change
@@ -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;
}
Expand All @@ -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();
84 changes: 84 additions & 0 deletions test/addons-napi/test_function/test_function.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,78 @@ static napi_value TestFunctionName(napi_env env, napi_callback_info info) {
return NULL;
}

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(
Expand All @@ -43,9 +115,21 @@ static 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;
}
Expand Down

0 comments on commit 109c599

Please sign in to comment.