Skip to content

Commit

Permalink
fix getting address to a temporary
Browse files Browse the repository at this point in the history
  • Loading branch information
vmoroz committed Apr 26, 2022
1 parent 174938a commit 68c0f2e
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 43 deletions.
61 changes: 30 additions & 31 deletions src/js_native_api_v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,46 +94,44 @@ struct FinalizerContext {
} // namespace
} // namespace v8impl

void napi_env__::CallFinalizer(node_api_native_data* native_data) noexcept {
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.finalizer_type == node_api_finalizer_uses_js) {
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.finalizer(
env, native_data.data, native_data.finalizer_state);
},
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.finalizer(env, native_data.data, native_data.finalizer_state);
});
}
}

void napi_env__::CallFinalizerAsync(
node_api_native_data* native_data) noexcept {
const node_api_native_data& native_data) noexcept {
// We need to keep the env live until the finalizer has been run
// EnvRefHolder provides an exception safe wrapper to Ref and then
// Unref once the lambda is freed
EnvRefHolder liveEnv(static_cast<napi_env>(this));
v8impl::SetImmediate(
this, [native_data_copy = *native_data, liveEnv = std::move(liveEnv)]() {
napi_env env = liveEnv.env();
v8::HandleScope handle_scope(env->isolate);
v8::Context::Scope context_scope(env->context());
env->CallIntoModule([&native_data_copy](napi_env env) {
native_data_copy.finalizer(
env, native_data_copy.data, native_data_copy.finalizer_state);
});
});
v8impl::SetImmediate(this, [native_data, liveEnv = std::move(liveEnv)]() {
napi_env env = liveEnv.env();
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);
});
});
}

v8impl::ErrorState napi_env__::ExchangeErrorState(
Expand Down Expand Up @@ -340,7 +338,7 @@ class CallbackBundle {
cbdata,
0,
true,
&MakeNativeData(
MakeNativeData(
bundle, Delete, nullptr, node_api_finalizer_native_only));
return cbdata;
}
Expand Down Expand Up @@ -525,11 +523,11 @@ inline napi_status Wrap(napi_env env,
// 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);
reference = v8impl::Reference::New(env, obj, 0, false, native_data);
reference = v8impl::Reference::New(env, obj, 0, false, *native_data);
*result = reinterpret_cast<napi_ref>(reference);
} else {
// Create a self-deleting reference.
reference = v8impl::Reference::New(env, obj, 0, true, native_data);
reference = v8impl::Reference::New(env, obj, 0, true, *native_data);
}

if (wrap_type == retrievable) {
Expand All @@ -548,18 +546,18 @@ inline napi_status Wrap(napi_env env,
RefBase::RefBase(napi_env env,
uint32_t initial_refcount,
bool delete_self,
node_api_native_data* native_data)
const node_api_native_data& native_data)
: Finalizer(env, native_data),
_refcount(initial_refcount),
_delete_self(delete_self) {
Link(native_data->finalizer == nullptr ? &env->reflist
: &env->finalizing_reflist);
Link(native_data.finalizer == nullptr ? &env->reflist
: &env->finalizing_reflist);
}

RefBase* RefBase::New(napi_env env,
uint32_t initial_refcount,
bool delete_self,
node_api_native_data* native_data) {
const node_api_native_data& native_data) {
return new RefBase(env, initial_refcount, delete_self, native_data);
}

Expand Down Expand Up @@ -636,12 +634,11 @@ void RefBase::Finalize(bool is_env_teardown) {

if (_finalize_callback != nullptr) {
// This ensures that we never call the finalizer twice.
node_api_native_data native_data =
_env->CallFinalizer(
MakeNativeData(_finalize_data,
std::exchange(_finalize_callback, nullptr),
_finalize_hint,
_finalizer_type);
_env->CallFinalizer(&native_data);
_finalizer_type));
}

// this is safe because if a request to delete the reference
Expand Down Expand Up @@ -669,7 +666,7 @@ Reference* Reference::New(napi_env env,
v8::Local<v8::Value> value,
uint32_t initial_refcount,
bool delete_self,
node_api_native_data* native_data) {
const node_api_native_data& native_data) {
return new Reference(env, value, initial_refcount, delete_self, native_data);
}

Expand Down Expand Up @@ -2459,7 +2456,7 @@ napi_status node_api_create_external(napi_env env,

// The Reference object will delete itself after invoking the finalizer
// callback.
v8impl::Reference::New(env, external_value, 0, true, native_data);
v8impl::Reference::New(env, external_value, 0, true, *native_data);

*result = v8impl::JsValueFromV8LocalValue(external_value);

Expand Down Expand Up @@ -2561,7 +2558,7 @@ napi_status napi_create_reference(napi_env env,
}

v8impl::Reference* reference = v8impl::Reference::New(
env, v8_value, initial_refcount, false, &node_api_native_data{});
env, v8_value, initial_refcount, false, node_api_native_data{});

*result = reinterpret_cast<napi_ref>(reference);
return napi_clear_last_error(env);
Expand Down Expand Up @@ -3281,7 +3278,9 @@ napi_status node_api_set_instance_data(napi_env env,
v8impl::RefBase::Delete(old_data);
}

env->instance_data = v8impl::RefBase::New(env, 0, true, native_data);
if (native_data != nullptr) {
env->instance_data = v8impl::RefBase::New(env, 0, true, *native_data);
}

return napi_clear_last_error(env);
}
Expand Down
22 changes: 11 additions & 11 deletions src/js_native_api_v8.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ struct napi_env__ {
}
}

void CallFinalizer(node_api_native_data* native_data) noexcept;
void CallFinalizerAsync(node_api_native_data* native_data) noexcept;
void CallFinalizer(const node_api_native_data& native_data) noexcept;
void CallFinalizerAsync(const node_api_native_data& native_data) noexcept;

v8impl::ErrorState ExchangeErrorState(v8impl::ErrorState&& errorState);

Expand Down Expand Up @@ -308,13 +308,13 @@ class Finalizer {

protected:
Finalizer(napi_env env,
node_api_native_data* native_data,
const node_api_native_data& native_data,
EnvReferenceMode refmode = kNoEnvReference)
: _env(env),
_finalize_callback(native_data->finalizer),
_finalize_data(native_data->data),
_finalize_hint(native_data->finalizer_state),
_finalizer_type(native_data->finalizer_type),
_finalize_callback(native_data.finalizer),
_finalize_data(native_data.data),
_finalize_hint(native_data.finalizer_state),
_finalizer_type(native_data.finalizer_type),
_has_env_reference(refmode == kKeepEnvReference) {
if (_has_env_reference) _env->Ref();
}
Expand All @@ -325,7 +325,7 @@ class Finalizer {

public:
static Finalizer* New(napi_env env,
node_api_native_data* native_data,
const node_api_native_data& native_data,
EnvReferenceMode refmode = kNoEnvReference) {
return new Finalizer(env, native_data, refmode);
}
Expand Down Expand Up @@ -362,13 +362,13 @@ class RefBase : protected Finalizer, RefTracker {
RefBase(napi_env env,
uint32_t initial_refcount,
bool delete_self,
node_api_native_data* native_data);
const node_api_native_data& native_data);

public:
static RefBase* New(napi_env env,
uint32_t initial_refcount,
bool delete_self,
node_api_native_data* native_data);
const node_api_native_data& native_data);

static inline void Delete(RefBase* reference);

Expand Down Expand Up @@ -398,7 +398,7 @@ class Reference : public RefBase {
v8::Local<v8::Value> value,
uint32_t initial_refcount,
bool delete_self,
node_api_native_data* native_data);
const node_api_native_data& native_data);

virtual ~Reference();
uint32_t Ref();
Expand Down
3 changes: 2 additions & 1 deletion src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,7 @@ napi_status node_api_create_external_buffer(napi_env env,
size_t length,
napi_value* result) {
NAPI_PREAMBLE(env);
CHECK_ARG(env, native_data);
CHECK_ARG(env, result);

v8::Isolate* isolate = env->isolate;
Expand All @@ -913,7 +914,7 @@ napi_status node_api_create_external_buffer(napi_env env,
node_api_native_data native_data_copy = *native_data;
native_data_copy.data = nullptr;
v8impl::Finalizer* finalizer = v8impl::Finalizer::New(
env, &native_data_copy, v8impl::Finalizer::kKeepEnvReference);
env, native_data_copy, v8impl::Finalizer::kKeepEnvReference);

v8::MaybeLocal<v8::Object> maybe =
node::Buffer::New(isolate,
Expand Down

0 comments on commit 68c0f2e

Please sign in to comment.