Commit 9648b06
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
5 files changed
Lines changed: 11 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -421,6 +421,9 @@ void FreeEnvironment(Environment* env) { | |||
| 421 | 421 | Context::Scope context_scope(env->context()); | |
| 422 | 422 | SealHandleScope seal_handle_scope(isolate); | |
| 423 | 423 | ||
| 424 | + // Set the flag in accordance with the DisallowJavascriptExecutionScope | ||
| 425 | + // above. | ||
| 426 | + env->set_can_call_into_js(false); | ||
| 424 | 427 | env->set_stopping(true); | |
| 425 | 428 | env->stop_sub_worker_contexts(); | |
| 426 | 429 | env->RunCleanup(); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -903,10 +903,13 @@ void Environment::InitializeLibuv() { | |||
| 903 | 903 | } | |
| 904 | 904 | ||
| 905 | 905 | void Environment::ExitEnv() { | |
| 906 | - set_can_call_into_js(false); | ||
| 906 | + // Should not access non-thread-safe methods here. | ||
| 907 | 907 | set_stopping(true); | |
| 908 | 908 | 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 | + }); | ||
| 910 | 913 | } | |
| 911 | 914 | ||
| 912 | 915 | void Environment::RegisterHandleCleanups() { | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -776,6 +776,7 @@ class Environment : public MemoryRetainer { | |||
| 776 | 776 | void stop_sub_worker_contexts(); | |
| 777 | 777 | template <typename Fn> | |
| 778 | 778 | inline void ForEachWorker(Fn&& iterator); | |
| 779 | + // Determine if the environment is stopping. This getter is thread-safe. | ||
| 779 | 780 | inline bool is_stopping() const; | |
| 780 | 781 | inline void set_stopping(bool value); | |
| 781 | 782 | inline std::list<node_module>* extra_linked_bindings(); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1127,7 +1127,7 @@ int Http2Session::OnStreamClose(nghttp2_session* handle, | |||
| 1127 | 1127 | // Don't close synchronously in case there's pending data to be written. This | |
| 1128 | 1128 | // may happen when writing trailing headers. | |
| 1129 | 1129 | if (code == NGHTTP2_NO_ERROR && nghttp2_session_want_write(handle) && | |
| 1130 | - !env->is_stopping()) { | ||
| 1130 | + env->can_call_into_js()) { | ||
| 1131 | 1131 | env->SetImmediate([handle, id, code, user_data](Environment* env) { | |
| 1132 | 1132 | OnStreamClose(handle, id, code, user_data); | |
| 1133 | 1133 | }); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -609,7 +609,7 @@ void ReportWritesToJSStreamListener::OnStreamAfterReqFinished( | |||
| 609 | 609 | StreamReq* req_wrap, int status) { | |
| 610 | 610 | StreamBase* stream = static_cast<StreamBase*>(stream_); | |
| 611 | 611 | Environment* env = stream->stream_env(); | |
| 612 | - if (env->is_stopping()) return; | ||
| 612 | + if (!env->can_call_into_js()) return; | ||
| 613 | 613 | AsyncWrap* async_wrap = req_wrap->GetAsyncWrap(); | |
| 614 | 614 | HandleScope handle_scope(env->isolate()); | |
| 615 | 615 | Context::Scope context_scope(env->context()); | |
0 commit comments