Skip to content

Commit 916f2c5

Browse files
santigimenojuanarbol
authored andcommitted
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 <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
1 parent 1308e68 commit 916f2c5

File tree

3 files changed

+31
-18
lines changed

3 files changed

+31
-18
lines changed

src/env.cc

+17-8
Original file line numberDiff line numberDiff line change
@@ -162,14 +162,17 @@ bool AsyncHooks::pop_async_context(double async_id) {
162162
}
163163

164164
void AsyncHooks::clear_async_id_stack() {
165-
Isolate* isolate = env()->isolate();
166-
HandleScope handle_scope(isolate);
167-
if (!js_execution_async_resources_.IsEmpty()) {
168-
USE(PersistentToLocal::Strong(js_execution_async_resources_)
169-
->Set(env()->context(),
170-
env()->length_string(),
171-
Integer::NewFromUnsigned(isolate, 0)));
165+
if (env()->can_call_into_js()) {
166+
Isolate* isolate = env()->isolate();
167+
HandleScope handle_scope(isolate);
168+
if (!js_execution_async_resources_.IsEmpty()) {
169+
USE(PersistentToLocal::Strong(js_execution_async_resources_)
170+
->Set(env()->context(),
171+
env()->length_string(),
172+
Integer::NewFromUnsigned(isolate, 0)));
173+
}
172174
}
175+
173176
native_execution_async_resources_.clear();
174177
native_execution_async_resources_.shrink_to_fit();
175178

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

11621171
size_t ref_count = 0;
11631172

src/node_http2.cc

+13-10
Original file line numberDiff line numberDiff line change
@@ -1120,13 +1120,14 @@ int Http2Session::OnStreamClose(nghttp2_session* handle,
11201120
// It is possible for the stream close to occur before the stream is
11211121
// ever passed on to the javascript side. If that happens, the callback
11221122
// will return false.
1123-
Local<Value> arg = Integer::NewFromUnsigned(isolate, code);
1124-
MaybeLocal<Value> answer =
1125-
stream->MakeCallback(env->http2session_on_stream_close_function(),
1126-
1, &arg);
1127-
if (answer.IsEmpty() || answer.ToLocalChecked()->IsFalse()) {
1128-
// Skip to destroy
1129-
stream->Destroy();
1123+
if (env->can_call_into_js()) {
1124+
Local<Value> arg = Integer::NewFromUnsigned(isolate, code);
1125+
MaybeLocal<Value> answer = stream->MakeCallback(
1126+
env->http2session_on_stream_close_function(), 1, &arg);
1127+
if (answer.IsEmpty() || answer.ToLocalChecked()->IsFalse()) {
1128+
// Skip to destroy
1129+
stream->Destroy();
1130+
}
11301131
}
11311132
return 0;
11321133
}
@@ -1629,9 +1630,11 @@ void Http2Session::MaybeScheduleWrite() {
16291630

16301631
// Sending data may call arbitrary JS code, so keep track of
16311632
// async context.
1632-
HandleScope handle_scope(env->isolate());
1633-
InternalCallbackScope callback_scope(this);
1634-
SendPendingData();
1633+
if (env->can_call_into_js()) {
1634+
HandleScope handle_scope(env->isolate());
1635+
InternalCallbackScope callback_scope(this);
1636+
SendPendingData();
1637+
}
16351638
});
16361639
}
16371640
}

src/node_platform.cc

+1
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,7 @@ int NodePlatform::NumberOfWorkerThreads() {
401401
}
402402

403403
void PerIsolatePlatformData::RunForegroundTask(std::unique_ptr<Task> task) {
404+
if (isolate_->IsExecutionTerminating()) return task->Run();
404405
DebugSealHandleScope scope(isolate_);
405406
Environment* env = Environment::GetCurrent(isolate_);
406407
if (env != nullptr) {

0 commit comments

Comments
 (0)