Skip to content

Commit c8fc5d0

Browse files
joyeecheungdanielleadams
authored andcommitted
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: #45282 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
1 parent 6f9d993 commit c8fc5d0

File tree

4 files changed

+42
-25
lines changed

4 files changed

+42
-25
lines changed

src/async_wrap.cc

+5-5
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,11 @@ static void SetupHooks(const FunctionCallbackInfo<Value>& args) {
185185
static void SetPromiseHooks(const FunctionCallbackInfo<Value>& args) {
186186
Environment* env = Environment::GetCurrent(args);
187187

188-
env->async_hooks()->SetJSPromiseHooks(
189-
args[0]->IsFunction() ? args[0].As<Function>() : Local<Function>(),
190-
args[1]->IsFunction() ? args[1].As<Function>() : Local<Function>(),
191-
args[2]->IsFunction() ? args[2].As<Function>() : Local<Function>(),
192-
args[3]->IsFunction() ? args[3].As<Function>() : Local<Function>());
188+
env->ResetPromiseHooks(
189+
args[0]->IsFunction() ? args[0].As<Function>() : Local<Function>(),
190+
args[1]->IsFunction() ? args[1].As<Function>() : Local<Function>(),
191+
args[2]->IsFunction() ? args[2].As<Function>() : Local<Function>(),
192+
args[3]->IsFunction() ? args[3].As<Function>() : Local<Function>());
193193
}
194194

195195
class DestroyParam {

src/env.cc

+27-12
Original file line numberDiff line numberDiff line change
@@ -63,20 +63,28 @@ int const ContextEmbedderTag::kNodeContextTag = 0x6e6f64;
6363
void* const ContextEmbedderTag::kNodeContextTagPtr = const_cast<void*>(
6464
static_cast<const void*>(&ContextEmbedderTag::kNodeContextTag));
6565

66-
void AsyncHooks::SetJSPromiseHooks(Local<Function> init,
66+
void AsyncHooks::ResetPromiseHooks(Local<Function> init,
6767
Local<Function> before,
6868
Local<Function> after,
6969
Local<Function> resolve) {
7070
js_promise_hooks_[0].Reset(env()->isolate(), init);
7171
js_promise_hooks_[1].Reset(env()->isolate(), before);
7272
js_promise_hooks_[2].Reset(env()->isolate(), after);
7373
js_promise_hooks_[3].Reset(env()->isolate(), resolve);
74+
}
75+
76+
void Environment::ResetPromiseHooks(Local<Function> init,
77+
Local<Function> before,
78+
Local<Function> after,
79+
Local<Function> resolve) {
80+
async_hooks()->ResetPromiseHooks(init, before, after, resolve);
81+
7482
for (auto it = contexts_.begin(); it != contexts_.end(); it++) {
7583
if (it->IsEmpty()) {
7684
contexts_.erase(it--);
7785
continue;
7886
}
79-
PersistentToLocal::Weak(env()->isolate(), *it)
87+
PersistentToLocal::Weak(isolate_, *it)
8088
->SetPromiseHooks(init, before, after, resolve);
8189
}
8290
}
@@ -179,7 +187,7 @@ void AsyncHooks::clear_async_id_stack() {
179187
fields_[kStackLength] = 0;
180188
}
181189

182-
void AsyncHooks::AddContext(Local<Context> ctx) {
190+
void AsyncHooks::InstallPromiseHooks(Local<Context> ctx) {
183191
ctx->SetPromiseHooks(js_promise_hooks_[0].IsEmpty()
184192
? Local<Function>()
185193
: PersistentToLocal::Strong(js_promise_hooks_[0]),
@@ -192,23 +200,24 @@ void AsyncHooks::AddContext(Local<Context> ctx) {
192200
js_promise_hooks_[3].IsEmpty()
193201
? Local<Function>()
194202
: PersistentToLocal::Strong(js_promise_hooks_[3]));
203+
}
195204

205+
void Environment::TrackContext(Local<Context> context) {
196206
size_t id = contexts_.size();
197207
contexts_.resize(id + 1);
198-
contexts_[id].Reset(env()->isolate(), ctx);
208+
contexts_[id].Reset(isolate_, context);
199209
contexts_[id].SetWeak();
200210
}
201211

202-
void AsyncHooks::RemoveContext(Local<Context> ctx) {
203-
Isolate* isolate = env()->isolate();
204-
HandleScope handle_scope(isolate);
212+
void Environment::UntrackContext(Local<Context> context) {
213+
HandleScope handle_scope(isolate_);
205214
contexts_.erase(std::remove_if(contexts_.begin(),
206215
contexts_.end(),
207216
[&](auto&& el) { return el.IsEmpty(); }),
208217
contexts_.end());
209218
for (auto it = contexts_.begin(); it != contexts_.end(); it++) {
210-
Local<Context> saved_context = PersistentToLocal::Weak(isolate, *it);
211-
if (saved_context == ctx) {
219+
Local<Context> saved_context = PersistentToLocal::Weak(isolate_, *it);
220+
if (saved_context == context) {
212221
it->Reset();
213222
contexts_.erase(it);
214223
break;
@@ -543,7 +552,8 @@ void Environment::AssignToContext(Local<v8::Context> context,
543552
inspector_agent()->ContextCreated(context, info);
544553
#endif // HAVE_INSPECTOR
545554

546-
this->async_hooks()->AddContext(context);
555+
this->async_hooks()->InstallPromiseHooks(context);
556+
TrackContext(context);
547557
}
548558

549559
void Environment::TryLoadAddon(
@@ -1466,8 +1476,9 @@ AsyncHooks::SerializeInfo AsyncHooks::Serialize(Local<Context> context,
14661476
context,
14671477
native_execution_async_resources_[i]);
14681478
}
1469-
CHECK_EQ(contexts_.size(), 1);
1470-
CHECK_EQ(contexts_[0], env()->context());
1479+
1480+
// At the moment, promise hooks are not supported in the startup snapshot.
1481+
// TODO(joyeecheung): support promise hooks in the startup snapshot.
14711482
CHECK(js_promise_hooks_[0].IsEmpty());
14721483
CHECK(js_promise_hooks_[1].IsEmpty());
14731484
CHECK(js_promise_hooks_[2].IsEmpty());
@@ -1601,6 +1612,10 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) {
16011612
should_abort_on_uncaught_toggle_.Serialize(ctx, creator);
16021613

16031614
info.principal_realm = principal_realm_->Serialize(creator);
1615+
// For now we only support serialization of the main context.
1616+
// TODO(joyeecheung): support de/serialization of vm contexts.
1617+
CHECK_EQ(contexts_.size(), 1);
1618+
CHECK_EQ(contexts_[0], context());
16041619
return info;
16051620
}
16061621

src/env.h

+9-6
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,8 @@ class AsyncHooks : public MemoryRetainer {
302302
// The `js_execution_async_resources` array contains the value in that case.
303303
inline v8::Local<v8::Object> native_execution_async_resource(size_t index);
304304

305-
void SetJSPromiseHooks(v8::Local<v8::Function> init,
305+
void InstallPromiseHooks(v8::Local<v8::Context> ctx);
306+
void ResetPromiseHooks(v8::Local<v8::Function> init,
306307
v8::Local<v8::Function> before,
307308
v8::Local<v8::Function> after,
308309
v8::Local<v8::Function> resolve);
@@ -321,9 +322,6 @@ class AsyncHooks : public MemoryRetainer {
321322
bool pop_async_context(double async_id);
322323
void clear_async_id_stack(); // Used in fatal exceptions.
323324

324-
void AddContext(v8::Local<v8::Context> ctx);
325-
void RemoveContext(v8::Local<v8::Context> ctx);
326-
327325
AsyncHooks(const AsyncHooks&) = delete;
328326
AsyncHooks& operator=(const AsyncHooks&) = delete;
329327
AsyncHooks(AsyncHooks&&) = delete;
@@ -386,8 +384,6 @@ class AsyncHooks : public MemoryRetainer {
386384
// Non-empty during deserialization
387385
const SerializeInfo* info_ = nullptr;
388386

389-
std::vector<v8::Global<v8::Context>> contexts_;
390-
391387
std::array<v8::Global<v8::Function>, 4> js_promise_hooks_;
392388
};
393389

@@ -696,9 +692,15 @@ class Environment : public MemoryRetainer {
696692
template <typename T, typename OnCloseCallback>
697693
inline void CloseHandle(T* handle, OnCloseCallback callback);
698694

695+
void ResetPromiseHooks(v8::Local<v8::Function> init,
696+
v8::Local<v8::Function> before,
697+
v8::Local<v8::Function> after,
698+
v8::Local<v8::Function> resolve);
699699
void AssignToContext(v8::Local<v8::Context> context,
700700
Realm* realm,
701701
const ContextInfo& info);
702+
void TrackContext(v8::Local<v8::Context> context);
703+
void UntrackContext(v8::Local<v8::Context> context);
702704

703705
void StartProfilerIdleNotifier();
704706

@@ -1140,6 +1142,7 @@ class Environment : public MemoryRetainer {
11401142

11411143
EnabledDebugList enabled_debug_list_;
11421144

1145+
std::vector<v8::Global<v8::Context>> contexts_;
11431146
std::list<node_module> extra_linked_bindings_;
11441147
Mutex extra_linked_bindings_mutex_;
11451148

src/node_contextify.cc

+1-2
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,7 @@ ContextifyContext::~ContextifyContext() {
164164
Isolate* isolate = env()->isolate();
165165
HandleScope scope(isolate);
166166

167-
env()->async_hooks()
168-
->RemoveContext(PersistentToLocal::Weak(isolate, context_));
167+
env()->UntrackContext(PersistentToLocal::Weak(isolate, context_));
169168
context_.Reset();
170169
}
171170

0 commit comments

Comments
 (0)