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
  • Loading branch information
santigimeno committed Sep 20, 2022
1 parent 4e97ad7 commit 9b39fbd
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 18 deletions.
26 changes: 18 additions & 8 deletions src/env.cc
Original file line number Diff line number Diff line change
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,14 @@ 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
24 changes: 14 additions & 10 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1120,13 +1120,15 @@ 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 +1631,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
2 changes: 2 additions & 0 deletions src/node_platform.cc
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,8 @@ 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 9b39fbd

Please sign in to comment.