1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

n-api: delete callback bundle via reference

Browse Source 
We should strive to use weak persistent references consistently
throughout the code, since using `v8::Persistent` directly results
in having to change many sites in the code when the way we use it
changes.

N-API uses `v8impl::Reference` internally when maintaining a weak
persistent reference is necessary. So far, `v8impl::CallbackBundle` was
using `v8::Persistent` directly in order to weakly reference the JS
function backed by a N-API callback.

The change introduced here reduces `v8impl::CallbackBundle` to a simple
structure and uses a `v8impl::Reference` to weakly reference the N-API
callback with which it is associated. The structure is freed by the
`napi_finalize` callback of the `v8impl::Reference`. This brings
N-API use of `v8::Persistent` completely under the `v8impl::Reference`
umbrella, rendering our use of weak references consistent.

PR-URL: https://github.com/nodejs/node/pull/29479
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This commit is contained in:
Gabriel Schulhof 2019-09-06 14:09:00 -07:00
parent ab05d43083
commit 1f4f2b488e
1 changed files with 6 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
src/js_native_api_v8.cc
View File

@ -379,27 +379,10 @@ inline static napi_status Unwrap(napi_env env,
// Ref: benchmark/misc/function_call
// Discussion (incl. perf. data): https://github.com/nodejs/node/pull/21072
struct CallbackBundle {
// Bind the lifecycle of `this` C++ object to a JavaScript object.
// We never delete a CallbackBundle C++ object directly.
void BindLifecycleTo(v8::Isolate* isolate, v8::Local<v8::Value> target) {
handle.Reset(isolate, target);
handle.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
}
napi_env env; // Necessary to invoke C++ NAPI callback
void* cb_data; // The user provided callback data
napi_callback function_or_getter;
napi_callback setter;
v8impl::Persistent<v8::Value> handle; // Die with this JavaScript object
private:
static void WeakCallback(v8::WeakCallbackInfo<CallbackBundle> const& info) {
// Use the "WeakCallback mechanism" to delete the C++ `bundle` object.
// This will be called when the v8::External containing `this` pointer
// is being GC-ed.
CallbackBundle* bundle = info.GetParameter();
delete bundle;
}
};
// Base class extended by classes that wrap V8 function and property callback
@ -580,6 +563,11 @@ class SetterCallbackWrapper
const v8::Local<v8::Value>& _value;
};
static void DeleteCallbackBundle(napi_env env, void* data, void* hint) {
CallbackBundle* bundle = static_cast<CallbackBundle*>(data);
delete bundle;
}
// Creates an object to be made available to the static function callback
// wrapper, used to retrieve the native callback function and data pointer.
static
@ -591,7 +579,7 @@ v8::Local<v8::Value> CreateFunctionCallbackData(napi_env env,
bundle->cb_data = data;
bundle->env = env;
v8::Local<v8::Value> cbdata = v8::External::New(env->isolate, bundle);
bundle->BindLifecycleTo(env->isolate, cbdata);
Reference::New(env, cbdata, 0, true, DeleteCallbackBundle, bundle, nullptr);
return cbdata;
}