From 9ac7df1c6a4a7bb50874d179116807da1b129607 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 7 Nov 2022 22:06:01 +0100 Subject: [PATCH] src: track contexts in the Environment instead of AsyncHooks This makes it easier to support the vm contexts in the startup snapshot. We now manage the promise hooks using references to the contexts from the Environment, and AsyncHooks only hold references to the hooks. PR-URL: https://github.com/nodejs/node/pull/45282 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Chengzhong Wu Reviewed-By: Santiago Gimeno Reviewed-By: Minwoo Jung --- src/async_wrap.cc | 10 +++++----- src/env.cc | 39 +++++++++++++++++++++++++++------------ src/env.h | 15 +++++++++------ src/node_contextify.cc | 3 +-- 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 9e76ad63ca625f..e22a5a6bcff1c5 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -185,11 +185,11 @@ static void SetupHooks(const FunctionCallbackInfo& args) { static void SetPromiseHooks(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - env->async_hooks()->SetJSPromiseHooks( - args[0]->IsFunction() ? args[0].As() : Local(), - args[1]->IsFunction() ? args[1].As() : Local(), - args[2]->IsFunction() ? args[2].As() : Local(), - args[3]->IsFunction() ? args[3].As() : Local()); + env->ResetPromiseHooks( + args[0]->IsFunction() ? args[0].As() : Local(), + args[1]->IsFunction() ? args[1].As() : Local(), + args[2]->IsFunction() ? args[2].As() : Local(), + args[3]->IsFunction() ? args[3].As() : Local()); } class DestroyParam { diff --git a/src/env.cc b/src/env.cc index c1e635ce77d2ef..b0613dfbc1bfa9 100644 --- a/src/env.cc +++ b/src/env.cc @@ -63,7 +63,7 @@ int const ContextEmbedderTag::kNodeContextTag = 0x6e6f64; void* const ContextEmbedderTag::kNodeContextTagPtr = const_cast( static_cast(&ContextEmbedderTag::kNodeContextTag)); -void AsyncHooks::SetJSPromiseHooks(Local init, +void AsyncHooks::ResetPromiseHooks(Local init, Local before, Local after, Local resolve) { @@ -71,12 +71,20 @@ void AsyncHooks::SetJSPromiseHooks(Local init, js_promise_hooks_[1].Reset(env()->isolate(), before); js_promise_hooks_[2].Reset(env()->isolate(), after); js_promise_hooks_[3].Reset(env()->isolate(), resolve); +} + +void Environment::ResetPromiseHooks(Local init, + Local before, + Local after, + Local resolve) { + async_hooks()->ResetPromiseHooks(init, before, after, resolve); + for (auto it = contexts_.begin(); it != contexts_.end(); it++) { if (it->IsEmpty()) { contexts_.erase(it--); continue; } - PersistentToLocal::Weak(env()->isolate(), *it) + PersistentToLocal::Weak(isolate_, *it) ->SetPromiseHooks(init, before, after, resolve); } } @@ -179,7 +187,7 @@ void AsyncHooks::clear_async_id_stack() { fields_[kStackLength] = 0; } -void AsyncHooks::AddContext(Local ctx) { +void AsyncHooks::InstallPromiseHooks(Local ctx) { ctx->SetPromiseHooks(js_promise_hooks_[0].IsEmpty() ? Local() : PersistentToLocal::Strong(js_promise_hooks_[0]), @@ -192,23 +200,24 @@ void AsyncHooks::AddContext(Local ctx) { js_promise_hooks_[3].IsEmpty() ? Local() : PersistentToLocal::Strong(js_promise_hooks_[3])); +} +void Environment::TrackContext(Local context) { size_t id = contexts_.size(); contexts_.resize(id + 1); - contexts_[id].Reset(env()->isolate(), ctx); + contexts_[id].Reset(isolate_, context); contexts_[id].SetWeak(); } -void AsyncHooks::RemoveContext(Local ctx) { - Isolate* isolate = env()->isolate(); - HandleScope handle_scope(isolate); +void Environment::UntrackContext(Local context) { + HandleScope handle_scope(isolate_); contexts_.erase(std::remove_if(contexts_.begin(), contexts_.end(), [&](auto&& el) { return el.IsEmpty(); }), contexts_.end()); for (auto it = contexts_.begin(); it != contexts_.end(); it++) { - Local saved_context = PersistentToLocal::Weak(isolate, *it); - if (saved_context == ctx) { + Local saved_context = PersistentToLocal::Weak(isolate_, *it); + if (saved_context == context) { it->Reset(); contexts_.erase(it); break; @@ -543,7 +552,8 @@ void Environment::AssignToContext(Local context, inspector_agent()->ContextCreated(context, info); #endif // HAVE_INSPECTOR - this->async_hooks()->AddContext(context); + this->async_hooks()->InstallPromiseHooks(context); + TrackContext(context); } void Environment::TryLoadAddon( @@ -1465,8 +1475,9 @@ AsyncHooks::SerializeInfo AsyncHooks::Serialize(Local context, context, native_execution_async_resources_[i]); } - CHECK_EQ(contexts_.size(), 1); - CHECK_EQ(contexts_[0], env()->context()); + + // At the moment, promise hooks are not supported in the startup snapshot. + // TODO(joyeecheung): support promise hooks in the startup snapshot. CHECK(js_promise_hooks_[0].IsEmpty()); CHECK(js_promise_hooks_[1].IsEmpty()); CHECK(js_promise_hooks_[2].IsEmpty()); @@ -1600,6 +1611,10 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) { should_abort_on_uncaught_toggle_.Serialize(ctx, creator); info.principal_realm = principal_realm_->Serialize(creator); + // For now we only support serialization of the main context. + // TODO(joyeecheung): support de/serialization of vm contexts. + CHECK_EQ(contexts_.size(), 1); + CHECK_EQ(contexts_[0], context()); return info; } diff --git a/src/env.h b/src/env.h index e484092bc2d0ed..e1f63e71fa9cec 100644 --- a/src/env.h +++ b/src/env.h @@ -302,7 +302,8 @@ class AsyncHooks : public MemoryRetainer { // The `js_execution_async_resources` array contains the value in that case. inline v8::Local native_execution_async_resource(size_t index); - void SetJSPromiseHooks(v8::Local init, + void InstallPromiseHooks(v8::Local ctx); + void ResetPromiseHooks(v8::Local init, v8::Local before, v8::Local after, v8::Local resolve); @@ -321,9 +322,6 @@ class AsyncHooks : public MemoryRetainer { bool pop_async_context(double async_id); void clear_async_id_stack(); // Used in fatal exceptions. - void AddContext(v8::Local ctx); - void RemoveContext(v8::Local ctx); - AsyncHooks(const AsyncHooks&) = delete; AsyncHooks& operator=(const AsyncHooks&) = delete; AsyncHooks(AsyncHooks&&) = delete; @@ -386,8 +384,6 @@ class AsyncHooks : public MemoryRetainer { // Non-empty during deserialization const SerializeInfo* info_ = nullptr; - std::vector> contexts_; - std::array, 4> js_promise_hooks_; }; @@ -696,9 +692,15 @@ class Environment : public MemoryRetainer { template inline void CloseHandle(T* handle, OnCloseCallback callback); + void ResetPromiseHooks(v8::Local init, + v8::Local before, + v8::Local after, + v8::Local resolve); void AssignToContext(v8::Local context, Realm* realm, const ContextInfo& info); + void TrackContext(v8::Local context); + void UntrackContext(v8::Local context); void StartProfilerIdleNotifier(); @@ -1130,6 +1132,7 @@ class Environment : public MemoryRetainer { EnabledDebugList enabled_debug_list_; + std::vector> contexts_; std::list extra_linked_bindings_; Mutex extra_linked_bindings_mutex_; diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 1bfa01c19059b4..45c5b33b634a22 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -164,8 +164,7 @@ ContextifyContext::~ContextifyContext() { Isolate* isolate = env()->isolate(); HandleScope scope(isolate); - env()->async_hooks() - ->RemoveContext(PersistentToLocal::Weak(isolate, context_)); + env()->UntrackContext(PersistentToLocal::Weak(isolate, context_)); context_.Reset(); }