Skip to content

Commit

Permalink
change public API names
Browse files Browse the repository at this point in the history
  • Loading branch information
vmoroz committed Apr 10, 2022
1 parent bdf261c commit babb75d
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 42 deletions.
13 changes: 7 additions & 6 deletions src/js_native_api_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,16 +156,17 @@ typedef struct {

#ifdef NAPI_EXPERIMENTAL
typedef enum {
node_api_finalizer_uses_js, // may touch GC and must run in SetImmediate
node_api_finalizer_native_only, // safe to run from GC finalizer
} node_api_finalizer_type;
node_api_native_data_flags_none = 0,
// may touch GC and must run in SetImmediate
node_api_native_data_flags_finalize_uses_js = 1 << 0
} node_api_native_data_flags;

typedef struct {
size_t version; // version of the struct - must be 0.
void* data;
napi_finalize finalizer;
void* finalizer_state;
node_api_finalizer_type finalizer_type;
napi_finalize finalize_cb;
void* finalize_hint;
node_api_native_data_flags flags;
} node_api_native_data;
#endif

Expand Down
57 changes: 34 additions & 23 deletions src/js_native_api_v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,23 +97,24 @@ struct FinalizerContext {
void napi_env__::CallFinalizer(
const node_api_native_data& native_data) noexcept {
if (refs > 0) {
if (native_data.finalizer_type == node_api_finalizer_uses_js) {
if ((native_data.flags & node_api_native_data_flags_finalize_uses_js) !=
0) {
CallFinalizerAsync(native_data);
} else {
// Run finalizers synchronously to allow release native objects as soon as
// possible. No JS calls are allowed.
v8impl::FinalizerContext finalizerContext(this);
CallIntoModule(
[native_data](napi_env env) {
native_data.finalizer(
env, native_data.data, native_data.finalizer_state);
native_data.finalize_cb(
env, native_data.data, native_data.finalize_hint);
},
v8impl::FinalizerContext::AbortOnError);
}
} else {
v8::HandleScope handle_scope(isolate);
CallIntoModule([native_data](napi_env env) {
native_data.finalizer(env, native_data.data, native_data.finalizer_state);
native_data.finalize_cb(env, native_data.data, native_data.finalize_hint);
});
}
}
Expand All @@ -129,7 +130,7 @@ void napi_env__::CallFinalizerAsync(
v8::HandleScope handle_scope(env->isolate);
v8::Context::Scope context_scope(env->context());
env->CallIntoModule([&native_data](napi_env env) {
native_data.finalizer(env, native_data.data, native_data.finalizer_state);
native_data.finalize_cb(env, native_data.data, native_data.finalize_hint);
});
});
}
Expand Down Expand Up @@ -333,13 +334,9 @@ class CallbackBundle {
bundle->env = env;

v8::Local<v8::Value> cbdata = v8::External::New(env->isolate, bundle);
Reference::New(
env,
cbdata,
0,
true,
MakeNativeData(
bundle, Delete, nullptr, node_api_finalizer_native_only));
node_api_native_data native_data = MakeNativeData(
bundle, Delete, nullptr, node_api_native_data_flags_none);
Reference::New(env, cbdata, 0, true, native_data);
return cbdata;
}

Expand Down Expand Up @@ -513,7 +510,7 @@ inline napi_status Wrap(napi_env env,
napi_invalid_arg);
} else if (wrap_type == anonymous) {
// If no finalize callback is provided, we error out.
CHECK_ARG(env, native_data->finalizer);
CHECK_ARG(env, native_data->finalize_cb);
}

v8impl::Reference* reference = nullptr;
Expand All @@ -522,7 +519,7 @@ inline napi_status Wrap(napi_env env,
// ONLY in response to the finalize callback invocation. (If it is deleted
// before then, then the finalize callback will never be invoked.)
// Therefore a finalize callback is required when returning a reference.
CHECK_ARG(env, native_data->finalizer);
CHECK_ARG(env, native_data->finalize_cb);
reference = v8impl::Reference::New(env, obj, 0, false, *native_data);
*result = reinterpret_cast<napi_ref>(reference);
} else {
Expand Down Expand Up @@ -550,8 +547,8 @@ RefBase::RefBase(napi_env env,
: Finalizer(env, native_data),
_refcount(initial_refcount),
_delete_self(delete_self) {
Link(native_data.finalizer == nullptr ? &env->reflist
: &env->finalizing_reflist);
Link(native_data.finalize_cb == nullptr ? &env->reflist
: &env->finalizing_reflist);
}

RefBase* RefBase::New(napi_env env,
Expand Down Expand Up @@ -638,7 +635,7 @@ void RefBase::Finalize(bool is_env_teardown) {
MakeNativeData(_finalize_data,
std::exchange(_finalize_callback, nullptr),
_finalize_hint,
_finalizer_type));
_finalize_flags));
}

// this is safe because if a request to delete the reference
Expand Down Expand Up @@ -2413,7 +2410,10 @@ napi_status napi_wrap(napi_env env,
void* finalize_hint,
napi_ref* result) {
node_api_native_data native_data =
v8impl::MakeNativeData(native_object, finalize_cb, finalize_hint);
v8impl::MakeNativeData(native_object,
finalize_cb,
finalize_hint,
node_api_native_data_flags_finalize_uses_js);
return v8impl::Wrap<v8impl::retrievable>(
env, js_object, &native_data, result);
}
Expand All @@ -2438,8 +2438,12 @@ napi_status napi_create_external(napi_env env,
napi_finalize finalize_cb,
void* finalize_hint,
napi_value* result) {
return node_api_create_external(
env, &v8impl::MakeNativeData(data, finalize_cb, finalize_hint), result);
node_api_native_data native_data =
v8impl::MakeNativeData(data,
finalize_cb,
finalize_hint,
node_api_native_data_flags_finalize_uses_js);
return node_api_create_external(env, &native_data, result);
}

napi_status node_api_create_external(napi_env env,
Expand Down Expand Up @@ -3236,7 +3240,10 @@ napi_status napi_add_finalizer(napi_env env,
void* finalize_hint,
napi_ref* result) {
node_api_native_data native_data =
v8impl::MakeNativeData(native_object, finalize_cb, finalize_hint);
v8impl::MakeNativeData(native_object,
finalize_cb,
finalize_hint,
node_api_native_data_flags_finalize_uses_js);
return v8impl::Wrap<v8impl::anonymous>(env, js_object, &native_data, result);
}

Expand All @@ -3263,8 +3270,12 @@ napi_status napi_set_instance_data(napi_env env,
void* data,
napi_finalize finalize_cb,
void* finalize_hint) {
return node_api_set_instance_data(
env, &v8impl::MakeNativeData(data, finalize_cb, finalize_hint));
node_api_native_data native_data =
v8impl::MakeNativeData(data,
finalize_cb,
finalize_hint,
node_api_native_data_flags_finalize_uses_js);
return node_api_set_instance_data(env, &native_data);
}

napi_status node_api_set_instance_data(napi_env env,
Expand Down
23 changes: 11 additions & 12 deletions src/js_native_api_v8.h
Original file line number Diff line number Diff line change
Expand Up @@ -311,10 +311,10 @@ class Finalizer {
const node_api_native_data& native_data,
EnvReferenceMode refmode = kNoEnvReference)
: _env(env),
_finalize_callback(native_data.finalizer),
_finalize_callback(native_data.finalize_cb),
_finalize_data(native_data.data),
_finalize_hint(native_data.finalizer_state),
_finalizer_type(native_data.finalizer_type),
_finalize_hint(native_data.finalize_hint),
_finalize_flags(native_data.flags),
_has_env_reference(refmode == kKeepEnvReference) {
if (_has_env_reference) _env->Ref();
}
Expand All @@ -337,7 +337,7 @@ class Finalizer {
napi_finalize _finalize_callback;
void* _finalize_data;
void* _finalize_hint;
node_api_finalizer_type _finalizer_type;
node_api_native_data_flags _finalize_flags;
bool _finalize_ran = false;
bool _has_env_reference = false;
};
Expand Down Expand Up @@ -424,16 +424,15 @@ class Reference : public RefBase {
FRIEND_TEST(JsNativeApiV8Test, Reference);
};

inline node_api_native_data MakeNativeData(
void* data,
napi_finalize finalizer,
void* finalizer_state,
node_api_finalizer_type finalizer_type = node_api_finalizer_uses_js) {
inline node_api_native_data MakeNativeData(void* data,
napi_finalize finalize_cb,
void* finalize_hint,
node_api_native_data_flags flags) {
node_api_native_data native_data{};
native_data.data = data;
native_data.finalizer = finalizer;
native_data.finalizer_state = finalizer_state;
native_data.finalizer_type = finalizer_type;
native_data.finalize_cb = finalize_cb;
native_data.finalize_hint = finalize_hint;
native_data.flags = flags;
return native_data;
}

Expand Down
5 changes: 4 additions & 1 deletion src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -896,7 +896,10 @@ napi_status napi_create_external_buffer(napi_env env,
void* finalize_hint,
napi_value* result) {
node_api_native_data native_data =
v8impl::MakeNativeData(data, finalize_cb, finalize_hint);
v8impl::MakeNativeData(data,
finalize_cb,
finalize_hint,
node_api_native_data_flags_finalize_uses_js);
return node_api_create_external_buffer(env, &native_data, length, result);
}

Expand Down

0 comments on commit babb75d

Please sign in to comment.