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: #45907 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
nodejs · Jan 20, 2023 · d3d62ed · d3d62ed
1 parent 1cfa2e6
commit d3d62ed
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 4 deletions.
diff --git 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
@@ -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
@@ -778,6 +778,7 @@ class Environment : public MemoryRetainer {
   void stop_sub_worker_contexts();
   template <typename Fn>
   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<node_module>* extra_linked_bindings();

diff --git 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
@@ -609,7 +609,7 @@ void ReportWritesToJSStreamListener::OnStreamAfterReqFinished(
     StreamReq* req_wrap, int status) {
   StreamBase* stream = static_cast<StreamBase*>(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());