Skip to content

Commit

Permalink
src: remove usage on ScriptCompiler::CompileFunctionInContext
Browse files Browse the repository at this point in the history
V8 APIs like HostImportModuleDynamicallyCallback and
ScriptCompiler::CompileFunction is moving away from ScriptOrModule.

Replaces ScriptCompiler::CompileFunctionInContext with
ScriptCompiler::CompileFunction to remove the usages on the optional
out param ScriptOrModule.

PR-URL: nodejs#44198
Fixes: nodejs/node-v8#214
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
legendecas authored and Fyko committed Sep 15, 2022
1 parent 429cb7a commit 9457b13
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 33 deletions.
22 changes: 11 additions & 11 deletions src/node_builtins.cc
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,9 @@ MaybeLocal<Function> BuiltinLoader::LookupAndCompileInternal(
ScriptCompiler::CachedData* cached_data = nullptr;
{
// Note: The lock here should not extend into the
// `CompileFunctionInContext()` call below, because this function may
// recurse if there is a syntax error during bootstrap (because the fatal
// exception handler is invoked, which may load built-in modules).
// `CompileFunction()` call below, because this function may recurse if
// there is a syntax error during bootstrap (because the fatal exception
// handler is invoked, which may load built-in modules).
Mutex::ScopedLock lock(code_cache_mutex_);
auto cache_it = code_cache_.find(id);
if (cache_it != code_cache_.end()) {
Expand All @@ -267,20 +267,20 @@ MaybeLocal<Function> BuiltinLoader::LookupAndCompileInternal(
has_cache ? "with" : "without");

MaybeLocal<Function> maybe_fun =
ScriptCompiler::CompileFunctionInContext(context,
&script_source,
parameters->size(),
parameters->data(),
0,
nullptr,
options);
ScriptCompiler::CompileFunction(context,
&script_source,
parameters->size(),
parameters->data(),
0,
nullptr,
options);

// This could fail when there are early errors in the built-in modules,
// e.g. the syntax errors
Local<Function> fun;
if (!maybe_fun.ToLocal(&fun)) {
// In the case of early errors, v8 is already capable of
// decorating the stack for us - note that we use CompileFunctionInContext
// decorating the stack for us - note that we use CompileFunction
// so there is no need to worry about wrappers.
return MaybeLocal<Function>();
}
Expand Down
27 changes: 14 additions & 13 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ using v8::PropertyHandlerFlags;
using v8::Script;
using v8::ScriptCompiler;
using v8::ScriptOrigin;
using v8::ScriptOrModule;
using v8::String;
using v8::Uint32;
using v8::UnboundScript;
Expand Down Expand Up @@ -1130,11 +1129,15 @@ void ContextifyContext::CompileFunction(
}
}

Local<ScriptOrModule> script;
MaybeLocal<Function> maybe_fn = ScriptCompiler::CompileFunctionInContext(
parsing_context, &source, params.size(), params.data(),
context_extensions.size(), context_extensions.data(), options,
v8::ScriptCompiler::NoCacheReason::kNoCacheNoReason, &script);
MaybeLocal<Function> maybe_fn = ScriptCompiler::CompileFunction(
parsing_context,
&source,
params.size(),
params.data(),
context_extensions.size(),
context_extensions.data(),
options,
v8::ScriptCompiler::NoCacheReason::kNoCacheNoReason);

Local<Function> fn;
if (!maybe_fn.ToLocal(&fn)) {
Expand All @@ -1150,7 +1153,7 @@ void ContextifyContext::CompileFunction(
context).ToLocal(&cache_key)) {
return;
}
CompiledFnEntry* entry = new CompiledFnEntry(env, cache_key, id, script);
CompiledFnEntry* entry = new CompiledFnEntry(env, cache_key, id, fn);
env->id_to_function_map.emplace(id, entry);

Local<Object> result = Object::New(isolate);
Expand Down Expand Up @@ -1196,16 +1199,14 @@ void CompiledFnEntry::WeakCallback(
CompiledFnEntry::CompiledFnEntry(Environment* env,
Local<Object> object,
uint32_t id,
Local<ScriptOrModule> script)
: BaseObject(env, object),
id_(id),
script_(env->isolate(), script) {
script_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
Local<Function> fn)
: BaseObject(env, object), id_(id), fn_(env->isolate(), fn) {
fn_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
}

CompiledFnEntry::~CompiledFnEntry() {
env()->id_to_function_map.erase(id_);
script_.ClearWeak();
fn_.ClearWeak();
}

static void StartSigintWatchdog(const FunctionCallbackInfo<Value>& args) {
Expand Down
4 changes: 2 additions & 2 deletions src/node_contextify.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,14 @@ class CompiledFnEntry final : public BaseObject {
CompiledFnEntry(Environment* env,
v8::Local<v8::Object> object,
uint32_t id,
v8::Local<v8::ScriptOrModule> script);
v8::Local<v8::Function> fn);
~CompiledFnEntry();

bool IsNotIndicativeOfMemoryLeakAtExit() const override { return true; }

private:
uint32_t id_;
v8::Global<v8::ScriptOrModule> script_;
v8::Global<v8::Function> fn_;

static void WeakCallback(const v8::WeakCallbackInfo<CompiledFnEntry>& data);
};
Expand Down
14 changes: 7 additions & 7 deletions src/node_snapshotable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1335,13 +1335,13 @@ void CompileSerializeMain(const FunctionCallbackInfo<Value>& args) {
};
ScriptCompiler::Source script_source(source, origin);
Local<Function> fn;
if (ScriptCompiler::CompileFunctionInContext(context,
&script_source,
parameters.size(),
parameters.data(),
0,
nullptr,
ScriptCompiler::kEagerCompile)
if (ScriptCompiler::CompileFunction(context,
&script_source,
parameters.size(),
parameters.data(),
0,
nullptr,
ScriptCompiler::kEagerCompile)
.ToLocal(&fn)) {
args.GetReturnValue().Set(fn);
}
Expand Down

0 comments on commit 9457b13

Please sign in to comment.