Skip to content

Commit d3d62ed

Browse files
legendecasRafaelGSS
authored andcommitted
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]>
1 parent 1cfa2e6 commit d3d62ed

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
@@ -909,10 +909,13 @@ void Environment::InitializeLibuv() {
909909
}
910910

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

918921
void Environment::RegisterHandleCleanups() {

src/env.h

+1
Original file line numberDiff line numberDiff line change
@@ -778,6 +778,7 @@ class Environment : public MemoryRetainer {
778778
void stop_sub_worker_contexts();
779779
template <typename Fn>
780780
inline void ForEachWorker(Fn&& iterator);
781+
// Determine if the environment is stopping. This getter is thread-safe.
781782
inline bool is_stopping() const;
782783
inline void set_stopping(bool value);
783784
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)