Skip to content

Commit 9648b06

Browse files
legendecasjuanarbol
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 <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent d6fc855 commit 9648b06

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
@@ -421,6 +421,9 @@ void FreeEnvironment(Environment* env) {
421421
Context::Scope context_scope(env->context());
422422
SealHandleScope seal_handle_scope(isolate);
423423

424+
// Set the flag in accordance with the DisallowJavascriptExecutionScope
425+
// above.
426+
env->set_can_call_into_js(false);
424427
env->set_stopping(true);
425428
env->stop_sub_worker_contexts();
426429
env->RunCleanup();

src/env.cc

+5-2
Original file line numberDiff line numberDiff line change
@@ -903,10 +903,13 @@ void Environment::InitializeLibuv() {
903903
}
904904

905905
void Environment::ExitEnv() {
906-
set_can_call_into_js(false);
906+
// Should not access non-thread-safe methods here.
907907
set_stopping(true);
908908
isolate_->TerminateExecution();
909-
SetImmediateThreadsafe([](Environment* env) { uv_stop(env->event_loop()); });
909+
SetImmediateThreadsafe([](Environment* env) {
910+
env->set_can_call_into_js(false);
911+
uv_stop(env->event_loop());
912+
});
910913
}
911914

912915
void Environment::RegisterHandleCleanups() {

src/env.h

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