diff --git a/src/README.md b/src/README.md index c8c647e872da3d..4471dd7721ad8c 100644 --- a/src/README.md +++ b/src/README.md @@ -151,6 +151,25 @@ is done executing. `Local` handles can only be allocated on the C++ stack. Most of the V8 API uses `Local` handles to work with JavaScript values or return them from functions. +Additionally, according to [V8 public API documentation][`v8::Local`], local handles +(`v8::Local`) should **never** be allocated on the heap. + +This disallows heap-allocated data structures containing instances of `v8::Local` + +For example: + +```cpp +// Don't do this +std::vector> v1; +``` + +Instead, it is recommended to use `v8::LocalVector` provided by V8 +for such scenarios: + +```cpp +v8::LocalVector v1(isolate); +``` + Whenever a `Local` handle is created, a `v8::HandleScope` or `v8::EscapableHandleScope` object must exist on the stack. The `Local` is then added to that scope and deleted along with it. @@ -1409,6 +1428,7 @@ static void GetUserInfo(const FunctionCallbackInfo& args) { [`v8.h` in Code Search]: https://cs.chromium.org/chromium/src/v8/include/v8.h [`v8.h` in Node.js]: https://github.com/nodejs/node/blob/HEAD/deps/v8/include/v8.h [`v8.h` in V8]: https://github.com/v8/v8/blob/HEAD/include/v8.h +[`v8::Local`]: https://v8.github.io/api/head/classv8_1_1Local.html [`vm` module]: https://nodejs.org/api/vm.html [binding function]: #binding-functions [cleanup hooks]: #cleanup-hooks diff --git a/src/node_contextify.cc b/src/node_contextify.cc index af05a2ca3e9208..df44ec1f1dc6e1 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -1166,7 +1166,7 @@ Maybe StoreCodeCacheResult( MaybeLocal CompileFunction(Local context, Local filename, Local content, - std::vector>* parameters) { + LocalVector* parameters) { ScriptOrigin script_origin(filename, 0, 0, true); ScriptCompiler::Source script_source(content, script_origin); @@ -1483,7 +1483,7 @@ void ContextifyContext::CompileFunction( Context::Scope scope(parsing_context); // Read context extensions from buffer - std::vector> context_extensions; + LocalVector context_extensions(isolate); if (!context_extensions_buf.IsEmpty()) { for (uint32_t n = 0; n < context_extensions_buf->Length(); n++) { Local val; @@ -1494,7 +1494,7 @@ void ContextifyContext::CompileFunction( } // Read params from params buffer - std::vector> params; + LocalVector params(isolate); if (!params_buf.IsEmpty()) { for (uint32_t n = 0; n < params_buf->Length(); n++) { Local val; @@ -1526,22 +1526,24 @@ void ContextifyContext::CompileFunction( args.GetReturnValue().Set(result); } -static std::vector> GetCJSParameters(IsolateData* data) { - return { - data->exports_string(), - data->require_string(), - data->module_string(), - data->__filename_string(), - data->__dirname_string(), - }; +static LocalVector GetCJSParameters(IsolateData* data) { + LocalVector result(data->isolate(), + { + data->exports_string(), + data->require_string(), + data->module_string(), + data->__filename_string(), + data->__dirname_string(), + }); + return result; } Local ContextifyContext::CompileFunctionAndCacheResult( Environment* env, Local parsing_context, ScriptCompiler::Source* source, - std::vector> params, - std::vector> context_extensions, + LocalVector params, + LocalVector context_extensions, ScriptCompiler::CompileOptions options, bool produce_cached_data, Local id_symbol, @@ -1677,7 +1679,7 @@ static MaybeLocal CompileFunctionForCJSLoader( options = ScriptCompiler::kConsumeCodeCache; } - std::vector> params; + LocalVector params(isolate); if (is_cjs_scope) { params = GetCJSParameters(env->isolate_data()); } diff --git a/src/node_contextify.h b/src/node_contextify.h index de69c22b0ebaed..ba964811bb6740 100644 --- a/src/node_contextify.h +++ b/src/node_contextify.h @@ -149,8 +149,8 @@ class ContextifyContext final : CPPGC_MIXIN(ContextifyContext) { Environment* env, v8::Local parsing_context, v8::ScriptCompiler::Source* source, - std::vector> params, - std::vector> context_extensions, + v8::LocalVector params, + v8::LocalVector context_extensions, v8::ScriptCompiler::CompileOptions options, bool produce_cached_data, v8::Local id_symbol, @@ -244,7 +244,7 @@ v8::MaybeLocal CompileFunction( v8::Local context, v8::Local filename, v8::Local content, - std::vector>* parameters); + v8::LocalVector* parameters); } // namespace contextify } // namespace node diff --git a/src/node_sea.cc b/src/node_sea.cc index fb9f933a19fa70..d8f2a65867f361 100644 --- a/src/node_sea.cc +++ b/src/node_sea.cc @@ -36,6 +36,7 @@ using v8::FunctionCallbackInfo; using v8::HandleScope; using v8::Isolate; using v8::Local; +using v8::LocalVector; using v8::MaybeLocal; using v8::NewStringType; using v8::Object; @@ -450,13 +451,15 @@ std::optional GenerateCodeCache(std::string_view main_path, return std::nullopt; } - std::vector> parameters = { - FIXED_ONE_BYTE_STRING(isolate, "exports"), - FIXED_ONE_BYTE_STRING(isolate, "require"), - FIXED_ONE_BYTE_STRING(isolate, "module"), - FIXED_ONE_BYTE_STRING(isolate, "__filename"), - FIXED_ONE_BYTE_STRING(isolate, "__dirname"), - }; + LocalVector parameters( + isolate, + { + FIXED_ONE_BYTE_STRING(isolate, "exports"), + FIXED_ONE_BYTE_STRING(isolate, "require"), + FIXED_ONE_BYTE_STRING(isolate, "module"), + FIXED_ONE_BYTE_STRING(isolate, "__filename"), + FIXED_ONE_BYTE_STRING(isolate, "__dirname"), + }); // TODO(RaisinTen): Using the V8 code cache prevents us from using `import()` // in the SEA code. Support it. diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index f9acb7b1d1618e..7207e68d127ae0 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -41,6 +41,7 @@ using v8::FunctionCallbackInfo; using v8::HandleScope; using v8::Isolate; using v8::Local; +using v8::LocalVector; using v8::Object; using v8::ObjectTemplate; using v8::SnapshotCreator; @@ -1479,11 +1480,13 @@ void CompileSerializeMain(const FunctionCallbackInfo& args) { Local context = isolate->GetCurrentContext(); // TODO(joyeecheung): do we need all of these? Maybe we would want a less // internal version of them. - std::vector> parameters = { - FIXED_ONE_BYTE_STRING(isolate, "require"), - FIXED_ONE_BYTE_STRING(isolate, "__filename"), - FIXED_ONE_BYTE_STRING(isolate, "__dirname"), - }; + LocalVector parameters( + isolate, + { + FIXED_ONE_BYTE_STRING(isolate, "require"), + FIXED_ONE_BYTE_STRING(isolate, "__filename"), + FIXED_ONE_BYTE_STRING(isolate, "__dirname"), + }); Local fn; if (contextify::CompileFunction(context, filename, source, ¶meters) .ToLocal(&fn)) {