Skip to content

Commit

Permalink
src: distinguish env stopping flags
Browse files Browse the repository at this point in the history
`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 <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
legendecas authored and RafaelGSS committed Jan 17, 2023
1 parent 03beacf commit fa863f7
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 4 deletions.
3 changes: 3 additions & 0 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
7 changes: 5 additions & 2 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
2 changes: 1 addition & 1 deletion src/stream_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down

0 comments on commit fa863f7

Please sign in to comment.