From fa863f73c9edc29709f3378ae1afd5d2d24ce4d6 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Sat, 14 Jan 2023 17:59:17 +0800 Subject: [PATCH] src: distinguish env stopping flags `Environment::FreeEnvironment` creates a `DisallowJavascriptExecutionScope`, so the flag `Environment::can_call_into_js()` should also be set as `false`. As `Environment::can_call_into_js_` is a simple boolean flag, it should not be accessed off-threads. PR-URL: https://github.com/nodejs/node/pull/45907 Reviewed-By: Anna Henningsen Reviewed-By: Matteo Collina --- src/api/environment.cc | 3 +++ src/env.cc | 7 +++++-- src/env.h | 1 + src/node_http2.cc | 2 +- src/stream_base.cc | 2 +- 5 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index 68af6d7483db75..51e1e33a20b5eb 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -424,6 +424,9 @@ void FreeEnvironment(Environment* env) { Context::Scope context_scope(env->context()); SealHandleScope seal_handle_scope(isolate); + // Set the flag in accordance with the DisallowJavascriptExecutionScope + // above. + env->set_can_call_into_js(false); env->set_stopping(true); env->stop_sub_worker_contexts(); env->RunCleanup(); diff --git a/src/env.cc b/src/env.cc index d26656aa59049c..ccccb2a0f1bc9c 100644 --- a/src/env.cc +++ b/src/env.cc @@ -909,10 +909,13 @@ void Environment::InitializeLibuv() { } void Environment::ExitEnv() { - set_can_call_into_js(false); + // Should not access non-thread-safe methods here. set_stopping(true); isolate_->TerminateExecution(); - SetImmediateThreadsafe([](Environment* env) { uv_stop(env->event_loop()); }); + SetImmediateThreadsafe([](Environment* env) { + env->set_can_call_into_js(false); + uv_stop(env->event_loop()); + }); } void Environment::RegisterHandleCleanups() { diff --git a/src/env.h b/src/env.h index 779c29c5181335..7eb119c7e87dfa 100644 --- a/src/env.h +++ b/src/env.h @@ -778,6 +778,7 @@ class Environment : public MemoryRetainer { void stop_sub_worker_contexts(); template inline void ForEachWorker(Fn&& iterator); + // Determine if the environment is stopping. This getter is thread-safe. inline bool is_stopping() const; inline void set_stopping(bool value); inline std::list* extra_linked_bindings(); diff --git a/src/node_http2.cc b/src/node_http2.cc index 032625f1e4f199..bb4057c9925676 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1127,7 +1127,7 @@ int Http2Session::OnStreamClose(nghttp2_session* handle, // Don't close synchronously in case there's pending data to be written. This // may happen when writing trailing headers. if (code == NGHTTP2_NO_ERROR && nghttp2_session_want_write(handle) && - !env->is_stopping()) { + env->can_call_into_js()) { env->SetImmediate([handle, id, code, user_data](Environment* env) { OnStreamClose(handle, id, code, user_data); }); diff --git a/src/stream_base.cc b/src/stream_base.cc index 31cbf8fa199f7f..06840e06b3d5a6 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -609,7 +609,7 @@ void ReportWritesToJSStreamListener::OnStreamAfterReqFinished( StreamReq* req_wrap, int status) { StreamBase* stream = static_cast(stream_); Environment* env = stream->stream_env(); - if (env->is_stopping()) return; + if (!env->can_call_into_js()) return; AsyncWrap* async_wrap = req_wrap->GetAsyncWrap(); HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context());