Skip to content

Commit

Permalink
src: use v8::LocalVector consistently with other minor cleanups
Browse files Browse the repository at this point in the history
V8 introduced `v8::LocalVector` somewhat recently as an alternative
to using `std::vector<v8::Local<T>>` to help ensure that Local handles
are handled correctly. This updates most (but not all) of our uses
of `std::vector<v8::Local<T>>` to use `v8::LocalVector<T>` with a few
other minor cleanups encountered along the way.
  • Loading branch information
jasnell committed Dec 31, 2024
1 parent 19d58c5 commit 35b1cac
Show file tree
Hide file tree
Showing 19 changed files with 123 additions and 98 deletions.
6 changes: 4 additions & 2 deletions src/cares_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ namespace cares_wrap {
using v8::Array;
using v8::Context;
using v8::EscapableHandleScope;
using v8::Exception;
using v8::FunctionCallbackInfo;
using v8::FunctionTemplate;
using v8::HandleScope;
Expand All @@ -68,6 +69,7 @@ using v8::Isolate;
using v8::Just;
using v8::JustVoid;
using v8::Local;
using v8::LocalVector;
using v8::Maybe;
using v8::Nothing;
using v8::Null;
Expand Down Expand Up @@ -159,7 +161,7 @@ void ares_sockstate_cb(void* data, ares_socket_t sock, int read, int write) {
Local<Array> HostentToNames(Environment* env, struct hostent* host) {
EscapableHandleScope scope(env->isolate());

std::vector<Local<Value>> names;
LocalVector<Value> names(env->isolate());

for (uint32_t i = 0; host->h_aliases[i] != nullptr; ++i)
names.emplace_back(OneByteString(env->isolate(), host->h_aliases[i]));
Expand Down Expand Up @@ -1577,7 +1579,7 @@ void ConvertIpv6StringToBuffer(const FunctionCallbackInfo<Value>& args) {
unsigned char dst[16]; // IPv6 addresses are 128 bits (16 bytes)

if (uv_inet_pton(AF_INET6, *ip, dst) != 0) {
isolate->ThrowException(v8::Exception::Error(
isolate->ThrowException(Exception::Error(
String::NewFromUtf8(isolate, "Invalid IPv6 address").ToLocalChecked()));
return;
}
Expand Down
3 changes: 2 additions & 1 deletion src/crypto/crypto_cipher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ using v8::HandleScope;
using v8::Int32;
using v8::Isolate;
using v8::Local;
using v8::LocalVector;
using v8::Object;
using v8::Uint32;
using v8::Value;
Expand Down Expand Up @@ -222,7 +223,7 @@ void CipherBase::GetSSLCiphers(const FunctionCallbackInfo<Value>& args) {
};

const int n = sk_SSL_CIPHER_num(ciphers);
std::vector<Local<Value>> arr(n + arraysize(TLS13_CIPHERS));
LocalVector<Value> arr(env->isolate(), n + arraysize(TLS13_CIPHERS));

for (int i = 0; i < n; ++i) {
const SSL_CIPHER* cipher = sk_SSL_CIPHER_value(ciphers, i);
Expand Down
3 changes: 2 additions & 1 deletion src/crypto/crypto_ec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ using v8::Int32;
using v8::Isolate;
using v8::JustVoid;
using v8::Local;
using v8::LocalVector;
using v8::Maybe;
using v8::MaybeLocal;
using v8::Nothing;
Expand Down Expand Up @@ -95,7 +96,7 @@ void ECDH::GetCurves(const FunctionCallbackInfo<Value>& args) {
std::vector<EC_builtin_curve> curves(num_curves);
CHECK_EQ(EC_get_builtin_curves(curves.data(), num_curves), num_curves);

std::vector<Local<Value>> arr(num_curves);
LocalVector<Value> arr(env->isolate(), num_curves);
std::transform(curves.begin(), curves.end(), arr.begin(), [env](auto& curve) {
return OneByteString(env->isolate(), OBJ_nid2sn(curve.nid));
});
Expand Down
7 changes: 4 additions & 3 deletions src/crypto/crypto_hash.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ using v8::Isolate;
using v8::Just;
using v8::JustVoid;
using v8::Local;
using v8::LocalVector;
using v8::Maybe;
using v8::MaybeLocal;
using v8::Name;
Expand Down Expand Up @@ -144,12 +145,12 @@ void Hash::GetCachedAliases(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();
Local<Context> context = args.GetIsolate()->GetCurrentContext();
Environment* env = Environment::GetCurrent(context);
std::vector<Local<Name>> names;
std::vector<Local<Value>> values;
size_t size = env->alias_to_md_id_map.size();
#if OPENSSL_VERSION_MAJOR >= 3
LocalVector<Name> names(isolate);
LocalVector<Value> values(isolate);
names.reserve(size);
values.reserve(size);
#if OPENSSL_VERSION_MAJOR >= 3
for (auto& [alias, id] : env->alias_to_md_id_map) {
names.push_back(OneByteString(isolate, alias.c_str(), alias.size()));
values.push_back(v8::Uint32::New(isolate, id));
Expand Down
5 changes: 3 additions & 2 deletions src/crypto/crypto_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ using v8::HandleScope;
using v8::Isolate;
using v8::JustVoid;
using v8::Local;
using v8::LocalVector;
using v8::Maybe;
using v8::MaybeLocal;
using v8::NewStringType;
Expand Down Expand Up @@ -236,7 +237,7 @@ MaybeLocal<Value> cryptoErrorListToException(
if (errors.size() > 1) {
CHECK(exception->IsObject());
Local<Object> exception_obj = exception.As<Object>();
std::vector<Local<Value>> stack(errors.size() - 1);
LocalVector<Value> stack(env->isolate(), errors.size() - 1);

// Iterate over all but the last error in the list.
auto current = errors.begin();
Expand All @@ -252,7 +253,7 @@ MaybeLocal<Value> cryptoErrorListToException(
}

Local<v8::Array> stackArray =
v8::Array::New(env->isolate(), &stack[0], stack.size());
v8::Array::New(env->isolate(), stack.data(), stack.size());

if (!exception_obj
->Set(env->context(), env->openssl_error_stack(), stackArray)
Expand Down
3 changes: 2 additions & 1 deletion src/internal_only_v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ using v8::FunctionCallbackInfo;
using v8::Global;
using v8::Isolate;
using v8::Local;
using v8::LocalVector;
using v8::Object;
using v8::Value;

Expand Down Expand Up @@ -56,7 +57,7 @@ void QueryObjects(const FunctionCallbackInfo<Value>& args) {
PrototypeChainHas prototype_chain_has(context, proto.As<Object>());
std::vector<Global<Object>> out;
isolate->GetHeapProfiler()->QueryObjects(context, &prototype_chain_has, &out);
std::vector<Local<Value>> result;
LocalVector<Value> result(isolate);
result.reserve(out.size());
for (size_t i = 0; i < out.size(); ++i) {
result.push_back(out[i].Get(isolate));
Expand Down
85 changes: 47 additions & 38 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,35 @@ using node::contextify::ContextifyContext;
using v8::Array;
using v8::ArrayBufferView;
using v8::Context;
using v8::Data;
using v8::EscapableHandleScope;
using v8::Exception;
using v8::FixedArray;
using v8::Function;
using v8::FunctionCallbackInfo;
using v8::FunctionTemplate;
using v8::Global;
using v8::HandleScope;
using v8::Int32;
using v8::Integer;
using v8::Isolate;
using v8::Just;
using v8::Local;
using v8::LocalVector;
using v8::Maybe;
using v8::MaybeLocal;
using v8::MemorySpan;
using v8::Message;
using v8::MicrotaskQueue;
using v8::Module;
using v8::ModuleRequest;
using v8::Name;
using v8::Null;
using v8::Object;
using v8::ObjectTemplate;
using v8::PrimitiveArray;
using v8::Promise;
using v8::PromiseRejectEvent;
using v8::ScriptCompiler;
using v8::ScriptOrigin;
using v8::String;
Expand Down Expand Up @@ -103,7 +113,7 @@ ModuleWrap* ModuleWrap::GetFromModule(Environment* env,
return nullptr;
}

v8::Maybe<bool> ModuleWrap::CheckUnsettledTopLevelAwait() {
Maybe<bool> ModuleWrap::CheckUnsettledTopLevelAwait() {
Isolate* isolate = env()->isolate();
Local<Context> context = env()->context();

Expand All @@ -115,17 +125,17 @@ v8::Maybe<bool> ModuleWrap::CheckUnsettledTopLevelAwait() {
Local<Module> module = module_.Get(isolate);
// It's a synthetic module, likely a facade wrapping CJS.
if (!module->IsSourceTextModule()) {
return v8::Just(true);
return Just(true);
}

if (!module->IsGraphAsync()) { // There is no TLA, no need to check.
return v8::Just(true);
return Just(true);
}

auto stalled_messages =
std::get<1>(module->GetStalledTopLevelAwaitMessages(isolate));
if (stalled_messages.empty()) {
return v8::Just(true);
return Just(true);
}

if (env()->options()->warnings) {
Expand All @@ -138,7 +148,7 @@ v8::Maybe<bool> ModuleWrap::CheckUnsettledTopLevelAwait() {
}
}

return v8::Just(false);
return Just(false);
}

Local<PrimitiveArray> ModuleWrap::GetHostDefinedOptions(
Expand Down Expand Up @@ -229,7 +239,7 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
Local<Array> export_names_arr = args[2].As<Array>();

uint32_t len = export_names_arr->Length();
std::vector<Local<String>> export_names(len);
LocalVector<String> export_names(realm->isolate(), len);
for (uint32_t i = 0; i < len; i++) {
Local<Value> export_name_val =
export_names_arr->Get(context, i).ToLocalChecked();
Expand All @@ -245,7 +255,7 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
// When we are compiling for the default loader, this will be
// std::nullopt, and CompileSourceTextModule() should use
// on-disk cache.
std::optional<v8::ScriptCompiler::CachedData*> user_cached_data;
std::optional<ScriptCompiler::CachedData*> user_cached_data;
if (id_symbol !=
realm->isolate_data()->source_text_module_default_hdo()) {
user_cached_data = nullptr;
Expand Down Expand Up @@ -324,7 +334,7 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
// be stored in an internal field.
Local<Object> context_object = context->GetExtrasBindingObject();
Local<Value> synthetic_evaluation_step =
synthetic ? args[3] : Undefined(realm->isolate()).As<v8::Value>();
synthetic ? args[3] : Undefined(realm->isolate()).As<Value>();

ModuleWrap* obj = new ModuleWrap(
realm, that, module, url, context_object, synthetic_evaluation_step);
Expand Down Expand Up @@ -405,22 +415,22 @@ static Local<Object> createImportAttributesContainer(
const int elements_per_attribute) {
CHECK_EQ(raw_attributes->Length() % elements_per_attribute, 0);
size_t num_attributes = raw_attributes->Length() / elements_per_attribute;
std::vector<Local<v8::Name>> names(num_attributes);
std::vector<Local<v8::Value>> values(num_attributes);
LocalVector<Name> names(isolate, num_attributes);
LocalVector<Value> values(isolate, num_attributes);

for (int i = 0; i < raw_attributes->Length(); i += elements_per_attribute) {
int idx = i / elements_per_attribute;
names[idx] = raw_attributes->Get(realm->context(), i).As<v8::Name>();
names[idx] = raw_attributes->Get(realm->context(), i).As<Name>();
values[idx] = raw_attributes->Get(realm->context(), i + 1).As<Value>();
}

return Object::New(
isolate, v8::Null(isolate), names.data(), values.data(), num_attributes);
isolate, Null(isolate), names.data(), values.data(), num_attributes);
}

static Local<Array> createModuleRequestsContainer(
Realm* realm, Isolate* isolate, Local<FixedArray> raw_requests) {
std::vector<Local<Value>> requests(raw_requests->Length());
LocalVector<Value> requests(isolate, raw_requests->Length());

for (int i = 0; i < raw_requests->Length(); i++) {
Local<ModuleRequest> module_request =
Expand All @@ -434,7 +444,7 @@ static Local<Array> createModuleRequestsContainer(
Local<Object> attributes =
createImportAttributesContainer(realm, isolate, raw_attributes, 3);

Local<v8::Name> names[] = {
Local<Name> names[] = {
realm->isolate_data()->specifier_string(),
realm->isolate_data()->attributes_string(),
};
Expand All @@ -444,8 +454,8 @@ static Local<Array> createModuleRequestsContainer(
};
DCHECK_EQ(arraysize(names), arraysize(values));

Local<Object> request = Object::New(
isolate, v8::Null(isolate), names, values, arraysize(names));
Local<Object> request =
Object::New(isolate, Null(isolate), names, values, arraysize(names));

requests[i] = request;
}
Expand Down Expand Up @@ -481,11 +491,11 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
Local<Array> modules = args[1].As<Array>();
CHECK_EQ(specifiers->Length(), modules->Length());

std::vector<v8::Global<Value>> specifiers_buffer;
std::vector<Global<Value>> specifiers_buffer;
if (FromV8Array(context, specifiers, &specifiers_buffer).IsNothing()) {
return;
}
std::vector<v8::Global<Value>> modules_buffer;
std::vector<Global<Value>> modules_buffer;
if (FromV8Array(context, modules, &modules_buffer).IsNothing()) {
return;
}
Expand Down Expand Up @@ -669,19 +679,18 @@ void ModuleWrap::EvaluateSync(const FunctionCallbackInfo<Value>& args) {
// before handler was added which would remove it from the unhandled
// rejection handling, since we are converting it into an error and throw
// from here directly.
Local<Value> type = v8::Integer::New(
isolate,
static_cast<int32_t>(
v8::PromiseRejectEvent::kPromiseHandlerAddedAfterReject));
Local<Value> type =
Integer::New(isolate,
static_cast<int32_t>(
PromiseRejectEvent::kPromiseHandlerAddedAfterReject));
Local<Value> args[] = {type, promise, Undefined(isolate)};
if (env->promise_reject_callback()
->Call(context, Undefined(isolate), arraysize(args), args)
.IsEmpty()) {
return;
}
Local<Value> exception = promise->Result();
Local<v8::Message> message =
v8::Exception::CreateMessage(isolate, exception);
Local<Message> message = Exception::CreateMessage(isolate, exception);
AppendExceptionLine(
env, exception, message, ErrorHandlingMode::MODULE_ERROR);
isolate->ThrowException(exception);
Expand Down Expand Up @@ -718,15 +727,15 @@ void ModuleWrap::GetNamespaceSync(const FunctionCallbackInfo<Value>& args) {
Local<Module> module = obj->module_.Get(isolate);

switch (module->GetStatus()) {
case v8::Module::Status::kUninstantiated:
case v8::Module::Status::kInstantiating:
case Module::Status::kUninstantiated:
case Module::Status::kInstantiating:
return realm->env()->ThrowError(
"Cannot get namespace, module has not been instantiated");
case v8::Module::Status::kInstantiated:
case v8::Module::Status::kEvaluated:
case v8::Module::Status::kErrored:
case Module::Status::kInstantiated:
case Module::Status::kEvaluated:
case Module::Status::kErrored:
break;
case v8::Module::Status::kEvaluating:
case Module::Status::kEvaluating:
UNREACHABLE();
}

Expand All @@ -746,14 +755,14 @@ void ModuleWrap::GetNamespace(const FunctionCallbackInfo<Value>& args) {
Local<Module> module = obj->module_.Get(isolate);

switch (module->GetStatus()) {
case v8::Module::Status::kUninstantiated:
case v8::Module::Status::kInstantiating:
case Module::Status::kUninstantiated:
case Module::Status::kInstantiating:
return realm->env()->ThrowError(
"cannot get namespace, module has not been instantiated");
case v8::Module::Status::kInstantiated:
case v8::Module::Status::kEvaluating:
case v8::Module::Status::kEvaluated:
case v8::Module::Status::kErrored:
case Module::Status::kInstantiated:
case Module::Status::kEvaluating:
case Module::Status::kEvaluated:
case Module::Status::kErrored:
break;
default:
UNREACHABLE();
Expand Down Expand Up @@ -825,7 +834,7 @@ MaybeLocal<Module> ModuleWrap::ResolveModuleCallback(

static MaybeLocal<Promise> ImportModuleDynamically(
Local<Context> context,
Local<v8::Data> host_defined_options,
Local<Data> host_defined_options,
Local<Value> resource_name,
Local<String> specifier,
Local<FixedArray> import_attributes) {
Expand Down Expand Up @@ -1011,7 +1020,7 @@ void ModuleWrap::CreateCachedData(const FunctionCallbackInfo<Value>& args) {

Local<Module> module = obj->module_.Get(isolate);

CHECK_LT(module->GetStatus(), v8::Module::Status::kEvaluating);
CHECK_LT(module->GetStatus(), Module::Status::kEvaluating);

Local<UnboundModuleScript> unbound_module_script =
module->GetUnboundModuleScript();
Expand Down
Loading

0 comments on commit 35b1cac

Please sign in to comment.