Skip to content
Permalink
Browse files
src: avoid using v8 on Isolate termination
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 RafaelGSS committed Sep 26, 2022
1 parent a6091f5 commit ce1704c2c740a40e578d92cd87b5e3986c6e52de
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 18 deletions.
@@ -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();

@@ -1068,7 +1071,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;

@@ -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;
}
@@ -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();
}
});
}
}
@@ -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) {

0 comments on commit ce1704c

Please sign in to comment.