Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 86e22b4

Browse filesBrowse files
joyeecheungRafaelGSS
joyeecheung
authored and
RafaelGSS
committed
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 <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
1 parent 717db1d commit 86e22b4
Copy full SHA for 86e22b4

File tree

Expand file treeCollapse file tree

4 files changed

+42
-25
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+42
-25
lines changed
Open diff view settings
Collapse file

‎src/async_wrap.cc‎

Copy file name to clipboardExpand all lines: src/async_wrap.cc
+5-5Lines changed: 5 additions & 5 deletions
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 {
Collapse file

‎src/env.cc‎

Copy file name to clipboardExpand all lines: src/env.cc
+27-12Lines changed: 27 additions & 12 deletions
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());
@@ -1602,6 +1613,10 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) {
16021613
should_abort_on_uncaught_toggle_.Serialize(ctx, creator);
16031614

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

Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+9-6Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,8 @@ class AsyncHooks : public MemoryRetainer {
303303
// The `js_execution_async_resources` array contains the value in that case.
304304
inline v8::Local<v8::Object> native_execution_async_resource(size_t index);
305305

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

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

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

@@ -701,9 +697,15 @@ class Environment : public MemoryRetainer {
701697
template <typename T, typename OnCloseCallback>
702698
inline void CloseHandle(T* handle, OnCloseCallback callback);
703699

700+
void ResetPromiseHooks(v8::Local<v8::Function> init,
701+
v8::Local<v8::Function> before,
702+
v8::Local<v8::Function> after,
703+
v8::Local<v8::Function> resolve);
704704
void AssignToContext(v8::Local<v8::Context> context,
705705
Realm* realm,
706706
const ContextInfo& info);
707+
void TrackContext(v8::Local<v8::Context> context);
708+
void UntrackContext(v8::Local<v8::Context> context);
707709

708710
void StartProfilerIdleNotifier();
709711

@@ -1145,6 +1147,7 @@ class Environment : public MemoryRetainer {
11451147

11461148
EnabledDebugList enabled_debug_list_;
11471149

1150+
std::vector<v8::Global<v8::Context>> contexts_;
11481151
std::list<node_module> extra_linked_bindings_;
11491152
Mutex extra_linked_bindings_mutex_;
11501153

Collapse file

‎src/node_contextify.cc‎

Copy file name to clipboardExpand all lines: src/node_contextify.cc
+1-2Lines changed: 1 addition & 2 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.