Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

node-api: type tag external values without v8::Private #51149

Merged
merged 1 commit into from
Dec 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 82 additions & 8 deletions src/js_native_api_v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,58 @@ void Reference::WeakCallback(const v8::WeakCallbackInfo<Reference>& data) {
reference->env_->InvokeFinalizerFromGC(reference);
}

/**
* A wrapper for `v8::External` to support type-tagging. `v8::External` doesn't
* support defining any properties and private properties on it, even though it
* is an object. This wrapper is used to store the type tag and the data of the
* external value.
*/
class ExternalWrapper {
private:
explicit ExternalWrapper(void* data) : data_(data) {}

static void WeakCallback(const v8::WeakCallbackInfo<ExternalWrapper>& data) {
ExternalWrapper* wrapper = data.GetParameter();
delete wrapper;
}

public:
static v8::Local<v8::External> New(napi_env env, void* data) {
ExternalWrapper* wrapper = new ExternalWrapper(data);
v8::Local<v8::External> external = v8::External::New(env->isolate, wrapper);
wrapper->persistent_.Reset(env->isolate, external);
wrapper->persistent_.SetWeak(
wrapper, WeakCallback, v8::WeakCallbackType::kParameter);

return external;
}

static ExternalWrapper* From(v8::Local<v8::External> external) {
return static_cast<ExternalWrapper*>(external->Value());
}

void* Data() { return data_; }

bool TypeTag(const napi_type_tag* type_tag) {
if (type_tag_ != nullptr) {
return false;
}
type_tag_ = type_tag;
return true;
}

bool CheckTypeTag(const napi_type_tag* type_tag) {
return type_tag == type_tag_ ||
(type_tag_ && type_tag->lower == type_tag_->lower &&
type_tag->upper == type_tag_->upper);
}

private:
v8impl::Persistent<v8::Value> persistent_;
void* data_;
const napi_type_tag* type_tag_ = nullptr;
};

} // end of namespace v8impl

// Warning: Keep in-sync with napi_status enum
Expand Down Expand Up @@ -2517,9 +2569,8 @@ napi_status NAPI_CDECL napi_create_external(napi_env env,
NAPI_PREAMBLE(env);
CHECK_ARG(env, result);

v8::Isolate* isolate = env->isolate;

v8::Local<v8::Value> external_value = v8::External::New(isolate, data);
v8::Local<v8::External> external_value =
v8impl::ExternalWrapper::New(env, data);

if (finalize_cb) {
// The Reference object will delete itself after invoking the finalizer
Expand All @@ -2539,12 +2590,24 @@ napi_status NAPI_CDECL napi_create_external(napi_env env,
}

napi_status NAPI_CDECL napi_type_tag_object(napi_env env,
napi_value object,
napi_value object_or_external,
const napi_type_tag* type_tag) {
NAPI_PREAMBLE(env);
v8::Local<v8::Context> context = env->context();

CHECK_ARG(env, object_or_external);
v8::Local<v8::Value> val =
v8impl::V8LocalValueFromJsValue(object_or_external);
if (val->IsExternal()) {
v8impl::ExternalWrapper* wrapper =
v8impl::ExternalWrapper::From(val.As<v8::External>());
RETURN_STATUS_IF_FALSE_WITH_PREAMBLE(
env, wrapper->TypeTag(type_tag), napi_invalid_arg);
return GET_RETURN_STATUS(env);
}

v8::Local<v8::Object> obj;
CHECK_TO_OBJECT_WITH_PREAMBLE(env, context, obj, object);
CHECK_TO_OBJECT_WITH_PREAMBLE(env, context, obj, object_or_external);
CHECK_ARG_WITH_PREAMBLE(env, type_tag);

auto key = NAPI_PRIVATE_KEY(context, type_tag);
Expand All @@ -2566,13 +2629,24 @@ napi_status NAPI_CDECL napi_type_tag_object(napi_env env,
}

napi_status NAPI_CDECL napi_check_object_type_tag(napi_env env,
napi_value object,
napi_value object_or_external,
const napi_type_tag* type_tag,
bool* result) {
NAPI_PREAMBLE(env);
v8::Local<v8::Context> context = env->context();

CHECK_ARG(env, object_or_external);
v8::Local<v8::Value> obj_val =
v8impl::V8LocalValueFromJsValue(object_or_external);
if (obj_val->IsExternal()) {
v8impl::ExternalWrapper* wrapper =
v8impl::ExternalWrapper::From(obj_val.As<v8::External>());
*result = wrapper->CheckTypeTag(type_tag);
return GET_RETURN_STATUS(env);
}

v8::Local<v8::Object> obj;
CHECK_TO_OBJECT_WITH_PREAMBLE(env, context, obj, object);
CHECK_TO_OBJECT_WITH_PREAMBLE(env, context, obj, object_or_external);
CHECK_ARG_WITH_PREAMBLE(env, type_tag);
CHECK_ARG_WITH_PREAMBLE(env, result);

Expand Down Expand Up @@ -2617,7 +2691,7 @@ napi_status NAPI_CDECL napi_get_value_external(napi_env env,
RETURN_STATUS_IF_FALSE(env, val->IsExternal(), napi_invalid_arg);

v8::Local<v8::External> external_value = val.As<v8::External>();
*result = external_value->Value();
*result = v8impl::ExternalWrapper::From(external_value)->Data();

return napi_clear_last_error(env);
}
Expand Down
11 changes: 6 additions & 5 deletions test/cctest/test_linked_binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,8 @@ napi_value NapiLinkedWithInstanceData(napi_env env, napi_value exports) {
napi_value key, value;
CHECK_EQ(napi_create_string_utf8(env, "hello", NAPI_AUTO_LENGTH, &key),
napi_ok);
CHECK_EQ(napi_create_external(env, instance_data, nullptr, nullptr, &value),
CHECK_EQ(napi_create_external_arraybuffer(
env, instance_data, 1, nullptr, nullptr, &value),
napi_ok);
CHECK_EQ(napi_set_property(env, exports, key, value), napi_ok);
return nullptr;
Expand Down Expand Up @@ -289,9 +290,9 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiInstanceDataTest) {
.ToLocalChecked();
v8::Local<v8::Value> completion_value =
script->Run(context).ToLocalChecked();
CHECK(completion_value->IsExternal());
CHECK(completion_value->IsArrayBuffer());
instance_data =
static_cast<int*>(completion_value.As<v8::External>()->Value());
static_cast<int*>(completion_value.As<v8::ArrayBuffer>()->Data());
CHECK_NE(instance_data, nullptr);
CHECK_EQ(*instance_data, 0);
}
Expand Down Expand Up @@ -327,9 +328,9 @@ TEST_F(LinkedBindingTest,
.ToLocalChecked();
v8::Local<v8::Value> completion_value =
script->Run(context).ToLocalChecked();
CHECK(completion_value->IsExternal());
CHECK(completion_value->IsArrayBuffer());
instance_data =
static_cast<int*>(completion_value.As<v8::External>()->Value());
static_cast<int*>(completion_value.As<v8::ArrayBuffer>()->Data());
CHECK_NE(instance_data, nullptr);
CHECK_EQ(*instance_data, 0);
}
Expand Down