Skip to content

Commit bb40507

Browse files
authored
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: nodejs#45907 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
1 parent be93b7a commit bb40507

File tree

5 files changed

+11
-4
lines changed

5 files changed

+11
-4
lines changed

src/api/environment.cc

+3
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,9 @@ void FreeEnvironment(Environment* env) {
424424
Context::Scope context_scope(env->context());
425425
SealHandleScope seal_handle_scope(isolate);
426426

427+
// Set the flag in accordance with the DisallowJavascriptExecutionScope
428+
// above.
429+
env->set_can_call_into_js(false);
427430
env->set_stopping(true);
428431
env->stop_sub_worker_contexts();
429432
env->RunCleanup();

src/env.cc

+5-2
Original file line numberDiff line numberDiff line change
@@ -910,10 +910,13 @@ void Environment::InitializeLibuv() {
910910
}
911911

912912
void Environment::ExitEnv() {
913-
set_can_call_into_js(false);
913+
// Should not access non-thread-safe methods here.
914914
set_stopping(true);
915915
isolate_->TerminateExecution();
916-
SetImmediateThreadsafe([](Environment* env) { uv_stop(env->event_loop()); });
916+
SetImmediateThreadsafe([](Environment* env) {
917+
env->set_can_call_into_js(false);
918+
uv_stop(env->event_loop());
919+
});
917920
}
918921

919922
void Environment::RegisterHandleCleanups() {

src/env.h

+1
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,7 @@ class Environment : public MemoryRetainer {
780780
void stop_sub_worker_contexts();
781781
template <typename Fn>
782782
inline void ForEachWorker(Fn&& iterator);
783+
// Determine if the environment is stopping. This getter is thread-safe.
783784
inline bool is_stopping() const;
784785
inline void set_stopping(bool value);
785786
inline std::list<node_module>* extra_linked_bindings();

src/node_http2.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -1127,7 +1127,7 @@ int Http2Session::OnStreamClose(nghttp2_session* handle,
11271127
// Don't close synchronously in case there's pending data to be written. This
11281128
// may happen when writing trailing headers.
11291129
if (code == NGHTTP2_NO_ERROR && nghttp2_session_want_write(handle) &&
1130-
!env->is_stopping()) {
1130+
env->can_call_into_js()) {
11311131
env->SetImmediate([handle, id, code, user_data](Environment* env) {
11321132
OnStreamClose(handle, id, code, user_data);
11331133
});

src/stream_base.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,7 @@ void ReportWritesToJSStreamListener::OnStreamAfterReqFinished(
609609
StreamReq* req_wrap, int status) {
610610
StreamBase* stream = static_cast<StreamBase*>(stream_);
611611
Environment* env = stream->stream_env();
612-
if (env->is_stopping()) return;
612+
if (!env->can_call_into_js()) return;
613613
AsyncWrap* async_wrap = req_wrap->GetAsyncWrap();
614614
HandleScope handle_scope(env->isolate());
615615
Context::Scope context_scope(env->context());

0 commit comments

Comments
 (0)