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

src: avoid using v8 on Isolate termination #44669

Merged
merged 2 commits into from Sep 21, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 17 additions & 8 deletions src/env.cc
Expand Up @@ -163,14 +163,17 @@ bool AsyncHooks::pop_async_context(double async_id) {
}

void AsyncHooks::clear_async_id_stack() {
Isolate* isolate = env()->isolate();
HandleScope handle_scope(isolate);
if (!js_execution_async_resources_.IsEmpty()) {
USE(PersistentToLocal::Strong(js_execution_async_resources_)
->Set(env()->context(),
env()->length_string(),
Integer::NewFromUnsigned(isolate, 0)));
if (env()->can_call_into_js()) {
Isolate* isolate = env()->isolate();
HandleScope handle_scope(isolate);
if (!js_execution_async_resources_.IsEmpty()) {
USE(PersistentToLocal::Strong(js_execution_async_resources_)
->Set(env()->context(),
env()->length_string(),
Integer::NewFromUnsigned(isolate, 0)));
}
}

native_execution_async_resources_.clear();
native_execution_async_resources_.shrink_to_fit();

Expand Down Expand Up @@ -1046,7 +1049,13 @@ void Environment::RunAndClearNativeImmediates(bool only_refed) {
TRACE_EVENT0(TRACING_CATEGORY_NODE1(environment),
"RunAndClearNativeImmediates");
HandleScope handle_scope(isolate_);
InternalCallbackScope cb_scope(this, Object::New(isolate_), { 0, 0 });
// In case the Isolate is no longer accessible just use an empty Local. This
// is not an issue for InternalCallbackScope as this case is already handled
// in its constructor but we avoid calls into v8 which can crash the process
// in debug builds.
Local<Object> obj =
can_call_into_js() ? Object::New(isolate_) : Local<Object>();
InternalCallbackScope cb_scope(this, obj, {0, 0});

size_t ref_count = 0;

Expand Down
23 changes: 13 additions & 10 deletions src/node_http2.cc
Expand Up @@ -1120,13 +1120,14 @@ int Http2Session::OnStreamClose(nghttp2_session* handle,
// It is possible for the stream close to occur before the stream is
// ever passed on to the javascript side. If that happens, the callback
// will return false.
Local<Value> arg = Integer::NewFromUnsigned(isolate, code);
MaybeLocal<Value> answer =
stream->MakeCallback(env->http2session_on_stream_close_function(),
1, &arg);
if (answer.IsEmpty() || answer.ToLocalChecked()->IsFalse()) {
// Skip to destroy
stream->Destroy();
if (env->can_call_into_js()) {
Local<Value> arg = Integer::NewFromUnsigned(isolate, code);
MaybeLocal<Value> answer = stream->MakeCallback(
env->http2session_on_stream_close_function(), 1, &arg);
if (answer.IsEmpty() || answer.ToLocalChecked()->IsFalse()) {
// Skip to destroy
stream->Destroy();
}
}
return 0;
}
Expand Down Expand Up @@ -1629,9 +1630,11 @@ void Http2Session::MaybeScheduleWrite() {

// Sending data may call arbitrary JS code, so keep track of
// async context.
HandleScope handle_scope(env->isolate());
InternalCallbackScope callback_scope(this);
SendPendingData();
if (env->can_call_into_js()) {
HandleScope handle_scope(env->isolate());
InternalCallbackScope callback_scope(this);
SendPendingData();
}
});
}
}
Expand Down
1 change: 1 addition & 0 deletions src/node_platform.cc
Expand Up @@ -406,6 +406,7 @@ int NodePlatform::NumberOfWorkerThreads() {
}

void PerIsolatePlatformData::RunForegroundTask(std::unique_ptr<Task> task) {
if (isolate_->IsExecutionTerminating()) return task->Run();
DebugSealHandleScope scope(isolate_);
Environment* env = Environment::GetCurrent(isolate_);
if (env != nullptr) {
Expand Down