Skip to content

Commit

Permalink
src: cache necessary isolate & context in api/*
Browse files Browse the repository at this point in the history 
Refs: #37473

PR-URL: #38366
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
@XadillaX @danielleadams
XadillaX authored and danielleadams committed Jun 2, 2021
1 parent 08ad2f6 commit 120849f
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 45 deletions.
28 changes: 17 additions & 11 deletions src/api/callback.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,11 @@ InternalCallbackScope::InternalCallbackScope(Environment* env,
return;
}

HandleScope handle_scope(env->isolate());
Isolate* isolate = env->isolate();

HandleScope handle_scope(isolate);
// If you hit this assertion, you forgot to enter the v8::Context first.
CHECK_EQ(Environment::GetCurrent(env->isolate()), env);
CHECK_EQ(Environment::GetCurrent(isolate), env);

env->isolate()->SetIdle(false);

Expand All @@ -83,7 +85,8 @@ void InternalCallbackScope::Close() {
if (closed_) return;
closed_ = true;

auto idle = OnScopeLeave([&]() { env_->isolate()->SetIdle(true); });
Isolate* isolate = env_->isolate();
auto idle = OnScopeLeave([&]() { isolate->SetIdle(true); });

if (!env_->can_call_into_js()) return;
auto perform_stopping_check = [&]() {
Expand Down Expand Up @@ -113,8 +116,9 @@ void InternalCallbackScope::Close() {

auto weakref_cleanup = OnScopeLeave([&]() { env_->RunWeakRefCleanup(); });

Local<Context> context = env_->context();
if (!tick_info->has_tick_scheduled()) {
env_->context()->GetMicrotaskQueue()->PerformCheckpoint(env_->isolate());
context->GetMicrotaskQueue()->PerformCheckpoint(isolate);

perform_stopping_check();
}
Expand All @@ -130,7 +134,7 @@ void InternalCallbackScope::Close() {
return;
}

HandleScope handle_scope(env_->isolate());
HandleScope handle_scope(isolate);
Local<Object> process = env_->process_object();

if (!env_->can_call_into_js()) return;
Expand All @@ -141,7 +145,7 @@ void InternalCallbackScope::Close() {
// to initializes the tick callback during bootstrap.
CHECK(!tick_callback.IsEmpty());

if (tick_callback->Call(env_->context(), process, 0, nullptr).IsEmpty()) {
if (tick_callback->Call(context, process, 0, nullptr).IsEmpty()) {
failed_ = true;
}
perform_stopping_check();
Expand Down Expand Up @@ -181,6 +185,7 @@ MaybeLocal<Value> InternalMakeCallback(Environment* env,

MaybeLocal<Value> ret;

Local<Context> context = env->context();
if (use_async_hooks_trampoline) {
MaybeStackBuffer<Local<Value>, 16> args(3 + argc);
args[0] = v8::Number::New(env->isolate(), asyncContext.async_id);
Expand All @@ -189,9 +194,9 @@ MaybeLocal<Value> InternalMakeCallback(Environment* env,
for (int i = 0; i < argc; i++) {
args[i + 3] = argv[i];
}
ret = hook_cb->Call(env->context(), recv, args.length(), &args[0]);
ret = hook_cb->Call(context, recv, args.length(), &args[0]);
} else {
ret = callback->Call(env->context(), recv, argc, argv);
ret = callback->Call(context, recv, argc, argv);
}

if (ret.IsEmpty()) {
Expand Down Expand Up @@ -266,7 +271,7 @@ MaybeLocal<Value> MakeCallback(Isolate* isolate,
if (ret.IsEmpty() && env->async_callback_scope_depth() == 0) {
// This is only for legacy compatibility and we may want to look into
// removing/adjusting it.
return Undefined(env->isolate());
return Undefined(isolate);
}
return ret;
}
Expand All @@ -285,11 +290,12 @@ MaybeLocal<Value> MakeSyncCallback(Isolate* isolate,
CHECK_NOT_NULL(env);
if (!env->can_call_into_js()) return Local<Value>();

Context::Scope context_scope(env->context());
Local<Context> context = env->context();
Context::Scope context_scope(context);
if (env->async_callback_scope_depth()) {
// There's another MakeCallback() on the stack, piggy back on it.
// In particular, retain the current async_context.
return callback->Call(env->context(), recv, argc, argv);
return callback->Call(context, recv, argc, argv);
}

// This is a toplevel invocation and the caller (intentionally)
Expand Down
7 changes: 4 additions & 3 deletions src/api/embed_helpers.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ Maybe<int> SpinEventLoop(Environment* env) {
MultiIsolatePlatform* platform = GetMultiIsolatePlatform(env);
CHECK_NOT_NULL(platform);

HandleScope handle_scope(env->isolate());
Isolate* isolate = env->isolate();
HandleScope handle_scope(isolate);
Context::Scope context_scope(env->context());
SealHandleScope seal(env->isolate());
SealHandleScope seal(isolate);

if (env->is_stopping()) return Nothing<int>();

Expand All @@ -35,7 +36,7 @@ Maybe<int> SpinEventLoop(Environment* env) {
uv_run(env->event_loop(), UV_RUN_DEFAULT);
if (env->is_stopping()) break;

platform->DrainTasks(env->isolate());
platform->DrainTasks(isolate);

more = uv_loop_alive(env->event_loop());
if (more && !env->is_stopping()) continue;
Expand Down
14 changes: 8 additions & 6 deletions src/api/environment.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,12 +359,13 @@ Environment* CreateEnvironment(
}

void FreeEnvironment(Environment* env) {
Isolate::DisallowJavascriptExecutionScope disallow_js(env->isolate(),
Isolate* isolate = env->isolate();
Isolate::DisallowJavascriptExecutionScope disallow_js(isolate,
Isolate::DisallowJavascriptExecutionScope::THROW_ON_FAILURE);
{
HandleScope handle_scope(env->isolate()); // For env->context().
HandleScope handle_scope(isolate); // For env->context().
Context::Scope context_scope(env->context());
SealHandleScope seal_handle_scope(env->isolate());
SealHandleScope seal_handle_scope(isolate);

env->set_stopping(true);
env->stop_sub_worker_contexts();
Expand All @@ -377,7 +378,7 @@ void FreeEnvironment(Environment* env) {
// NodePlatform implementation.
MultiIsolatePlatform* platform = env->isolate_data()->platform();
if (platform != nullptr)
platform->DrainTasks(env->isolate());
platform->DrainTasks(isolate);

delete env;
}
Expand Down Expand Up @@ -409,14 +410,15 @@ MaybeLocal<Value> LoadEnvironment(
Environment* env,
const char* main_script_source_utf8) {
CHECK_NOT_NULL(main_script_source_utf8);
Isolate* isolate = env->isolate();
return LoadEnvironment(
env,
[&](const StartExecutionCallbackInfo& info) -> MaybeLocal<Value> {
// This is a slightly hacky way to convert UTF-8 to UTF-16.
Local<String> str =
String::NewFromUtf8(env->isolate(),
String::NewFromUtf8(isolate,
main_script_source_utf8).ToLocalChecked();
auto main_utf16 = std::make_unique<String::Value>(env->isolate(), str);
auto main_utf16 = std::make_unique<String::Value>(isolate, str);

// TODO(addaleax): Avoid having a global table for all scripts.
std::string name = "embedder_main_" + std::to_string(env->thread_id());
Expand Down
28 changes: 16 additions & 12 deletions src/api/exceptions.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace node {

using v8::Context;
using v8::Exception;
using v8::Integer;
using v8::Isolate;
Expand Down Expand Up @@ -51,18 +52,19 @@ Local<Value> ErrnoException(Isolate* isolate,
}
e = Exception::Error(cons);

Local<Context> context = env->context();
Local<Object> obj = e.As<Object>();
obj->Set(env->context(),
obj->Set(context,
env->errno_string(),
Integer::New(isolate, errorno)).Check();
obj->Set(env->context(), env->code_string(), estring).Check();
obj->Set(context, env->code_string(), estring).Check();

if (path_string.IsEmpty() == false) {
obj->Set(env->context(), env->path_string(), path_string).Check();
obj->Set(context, env->path_string(), path_string).Check();
}

if (syscall != nullptr) {
obj->Set(env->context(),
obj->Set(context,
env->syscall_string(),
OneByteString(isolate, syscall)).Check();
}
Expand Down Expand Up @@ -135,15 +137,16 @@ Local<Value> UVException(Isolate* isolate,
Exception::Error(js_msg)->ToObject(isolate->GetCurrentContext())
.ToLocalChecked();

e->Set(env->context(),
Local<Context> context = env->context();
e->Set(context,
env->errno_string(),
Integer::New(isolate, errorno)).Check();
e->Set(env->context(), env->code_string(), js_code).Check();
e->Set(env->context(), env->syscall_string(), js_syscall).Check();
e->Set(context, env->code_string(), js_code).Check();
e->Set(context, env->syscall_string(), js_syscall).Check();
if (!js_path.IsEmpty())
e->Set(env->context(), env->path_string(), js_path).Check();
e->Set(context, env->path_string(), js_path).Check();
if (!js_dest.IsEmpty())
e->Set(env->context(), env->dest_string(), js_dest).Check();
e->Set(context, env->dest_string(), js_dest).Check();

return e;
}
Expand Down Expand Up @@ -209,19 +212,20 @@ Local<Value> WinapiErrnoException(Isolate* isolate,
e = Exception::Error(message);
}

Local<Context> context = env->context();
Local<Object> obj = e.As<Object>();
obj->Set(env->context(), env->errno_string(), Integer::New(isolate, errorno))
obj->Set(context, env->errno_string(), Integer::New(isolate, errorno))
.Check();

if (path != nullptr) {
obj->Set(env->context(),
obj->Set(context,
env->path_string(),
String::NewFromUtf8(isolate, path).ToLocalChecked())
.Check();
}

if (syscall != nullptr) {
obj->Set(env->context(),
obj->Set(context,
env->syscall_string(),
OneByteString(isolate, syscall))
.Check();
Expand Down
29 changes: 16 additions & 13 deletions src/api/hooks.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,15 @@ Maybe<bool> EmitProcessBeforeExit(Environment* env) {
AsyncWrap::DestroyAsyncIdsCallback(env);

HandleScope handle_scope(env->isolate());
Context::Scope context_scope(env->context());
Local<Context> context = env->context();
Context::Scope context_scope(context);

Local<Value> exit_code_v;
if (!env->process_object()->Get(env->context(), env->exit_code_string())
if (!env->process_object()->Get(context, env->exit_code_string())
.ToLocal(&exit_code_v)) return Nothing<bool>();

Local<Integer> exit_code;
if (!exit_code_v->ToInteger(env->context()).ToLocal(&exit_code)) {
if (!exit_code_v->ToInteger(context).ToLocal(&exit_code)) {
return Nothing<bool>();
}

Expand All @@ -59,28 +60,30 @@ int EmitExit(Environment* env) {

Maybe<int> EmitProcessExit(Environment* env) {
// process.emit('exit')
HandleScope handle_scope(env->isolate());
Context::Scope context_scope(env->context());
Isolate* isolate = env->isolate();
HandleScope handle_scope(isolate);
Local<Context> context = env->context();
Context::Scope context_scope(context);
Local<Object> process_object = env->process_object();

// TODO(addaleax): It might be nice to share process._exiting and
// process.exitCode via getter/setter pairs that pass data directly to the
// native side, so that we don't manually have to read and write JS properties
// here. These getters could use e.g. a typed array for performance.
if (process_object
->Set(env->context(),
FIXED_ONE_BYTE_STRING(env->isolate(), "_exiting"),
True(env->isolate())).IsNothing()) return Nothing<int>();
->Set(context,
FIXED_ONE_BYTE_STRING(isolate, "_exiting"),
True(isolate)).IsNothing()) return Nothing<int>();

Local<String> exit_code = env->exit_code_string();
Local<Value> code_v;
int code;
if (!process_object->Get(env->context(), exit_code).ToLocal(&code_v) ||
!code_v->Int32Value(env->context()).To(&code) ||
ProcessEmit(env, "exit", Integer::New(env->isolate(), code)).IsEmpty() ||
if (!process_object->Get(context, exit_code).ToLocal(&code_v) ||
!code_v->Int32Value(context).To(&code) ||
ProcessEmit(env, "exit", Integer::New(isolate, code)).IsEmpty() ||
// Reload exit code, it may be changed by `emit('exit')`
!process_object->Get(env->context(), exit_code).ToLocal(&code_v) ||
!code_v->Int32Value(env->context()).To(&code)) {
!process_object->Get(context, exit_code).ToLocal(&code_v) ||
!code_v->Int32Value(context).To(&code)) {
return Nothing<int>();
}

Expand Down

0 comments on commit 120849f

Please sign in to comment.