Skip to content

Commit

Permalink
src: avoid using v8 on Isolate termination
Browse files Browse the repository at this point in the history
Fix multiple instances of those uncovered while running the tests on
debug builds.

Fixes: nodejs/node-v8#227
PR-URL: #44669
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
santigimeno authored and juanarbol committed Oct 4, 2022
1 parent 3479eca commit f3ba106
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 18 deletions.
25 changes: 17 additions & 8 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -162,14 +162,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 @@ -1157,7 +1160,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,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

0 comments on commit f3ba106

Please sign in to comment.