FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: set PromiseHooks by Environment · nodejs/node@7ba3055 · GitHub

/ node Public

Commit 7ba3055

Browse files
bengl
authored and
danielleadams
committed
src: set PromiseHooks by Environment
The new JS PromiseHooks introduced in the referenced PR are per v8::Context. This meant that code depending on them, such as AsyncLocalStorage, wouldn't behave correctly across vm.Context instances. PromiseHooks are now synchronized across the main Context and any Context created via vm.Context. Refs: #36394 Fixes: #38781 Signed-off-by: Bryan English <bryan@bryanenglish.com> PR-URL: #38821 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent f162896 commit 7ba3055

6 files changed

Lines changed: 112 additions & 2 deletions

File tree

‎src/async_wrap.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -454,8 +454,8 @@ static void EnablePromiseHook(const FunctionCallbackInfo<Value>& args) {
454454

455455
static void SetPromiseHooks(const FunctionCallbackInfo<Value>& args) {
456456
Environment* env = Environment::GetCurrent(args);
457-
Local<Context> ctx = env->context();
458-
ctx->SetPromiseHooks(
457+
458+
env->async_hooks()->SetJSPromiseHooks(
459459
args[0]->IsFunction() ? args[0].As<Function>() : Local<Function>(),
460460
args[1]->IsFunction() ? args[1].As<Function>() : Local<Function>(),
461461
args[2]->IsFunction() ? args[2].As<Function>() : Local<Function>(),

‎src/env-inl.h‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,20 @@ v8::Local<v8::Object> AsyncHooks::native_execution_async_resource(size_t i) {
9595
return PersistentToLocal::Strong(native_execution_async_resources_[i]);
9696
}
9797

98+
inline void AsyncHooks::SetJSPromiseHooks(v8::Local<v8::Function> init,
99+
v8::Local<v8::Function> before,
100+
v8::Local<v8::Function> after,
101+
v8::Local<v8::Function> resolve) {
102+
js_promise_hooks_[0].Reset(env()->isolate(), init);
103+
js_promise_hooks_[1].Reset(env()->isolate(), before);
104+
js_promise_hooks_[2].Reset(env()->isolate(), after);
105+
js_promise_hooks_[3].Reset(env()->isolate(), resolve);
106+
for (auto it = contexts_.begin(); it != contexts_.end(); it++) {
107+
PersistentToLocal::Weak(env()->isolate(), *it)
108+
->SetPromiseHooks(init, before, after, resolve);
109+
}
110+
}
111+
98112
inline v8::Local<v8::String> AsyncHooks::provider_string(int idx) {
99113
return env()->isolate_data()->async_wrap_provider(idx);
100114
}
@@ -217,6 +231,41 @@ void AsyncHooks::clear_async_id_stack() {
217231
fields_[kStackLength] = 0;
218232
}
219233

234+
inline void AsyncHooks::AddContext(v8::Local<v8::Context> ctx) {
235+
ctx->SetPromiseHooks(
236+
js_promise_hooks_[0].IsEmpty() ?
237+
v8::Local<v8::Function>() :
238+
PersistentToLocal::Strong(js_promise_hooks_[0]),
239+
js_promise_hooks_[1].IsEmpty() ?
240+
v8::Local<v8::Function>() :
241+
PersistentToLocal::Strong(js_promise_hooks_[1]),
242+
js_promise_hooks_[2].IsEmpty() ?
243+
v8::Local<v8::Function>() :
244+
PersistentToLocal::Strong(js_promise_hooks_[2]),
245+
js_promise_hooks_[3].IsEmpty() ?
246+
v8::Local<v8::Function>() :
247+
PersistentToLocal::Strong(js_promise_hooks_[3]));
248+
249+
size_t id = contexts_.size();
250+
contexts_.resize(id + 1);
251+
contexts_[id].Reset(env()->isolate(), ctx);
252+
contexts_[id].SetWeak();
253+
}
254+
255+
inline void AsyncHooks::RemoveContext(v8::Local<v8::Context> ctx) {
256+
v8::Isolate* isolate = env()->isolate();
257+
v8::HandleScope handle_scope(isolate);
258+
for (auto it = contexts_.begin(); it != contexts_.end(); it++) {
259+
v8::Local<v8::Context> saved_context =
260+
PersistentToLocal::Weak(env()->isolate(), *it);
261+
if (saved_context == ctx) {
262+
it->Reset();
263+
contexts_.erase(it);
264+
break;
265+
}
266+
}
267+
}
268+
220269
// The DefaultTriggerAsyncIdScope(AsyncWrap*) constructor is defined in
221270
// async_wrap-inl.h to avoid a circular dependency.
222271

@@ -304,6 +353,8 @@ inline void Environment::AssignToContext(v8::Local<v8::Context> context,
304353
#if HAVE_INSPECTOR
305354
inspector_agent()->ContextCreated(context, info);
306355
#endif // HAVE_INSPECTOR
356+
357+
this->async_hooks()->AddContext(context);
307358
}
308359

309360
inline Environment* Environment::GetCurrent(v8::Isolate* isolate) {

‎src/env.cc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,6 +1156,12 @@ AsyncHooks::SerializeInfo AsyncHooks::Serialize(Local<Context> context,
11561156
context,
11571157
native_execution_async_resources_[i].Get(context->GetIsolate()));
11581158
}
1159+
CHECK_EQ(contexts_.size(), 1);
1160+
CHECK_EQ(contexts_[0], env()->context());
1161+
CHECK(js_promise_hooks_[0].IsEmpty());
1162+
CHECK(js_promise_hooks_[1].IsEmpty());
1163+
CHECK(js_promise_hooks_[2].IsEmpty());
1164+
CHECK(js_promise_hooks_[3].IsEmpty());
11591165

11601166
return info;
11611167
}
@@ -1164,6 +1170,7 @@ void AsyncHooks::MemoryInfo(MemoryTracker* tracker) const {
11641170
tracker->TrackField("async_ids_stack", async_ids_stack_);
11651171
tracker->TrackField("fields", fields_);
11661172
tracker->TrackField("async_id_fields", async_id_fields_);
1173+
tracker->TrackField("js_promise_hooks", js_promise_hooks_);
11671174
}
11681175

11691176
void AsyncHooks::grow_async_ids_stack() {

‎src/env.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,11 @@ class AsyncHooks : public MemoryRetainer {
701701
// The `js_execution_async_resources` array contains the value in that case.
702702
inline v8::Local<v8::Object> native_execution_async_resource(size_t index);
703703

704+
inline void SetJSPromiseHooks(v8::Local<v8::Function> init,
705+
v8::Local<v8::Function> before,
706+
v8::Local<v8::Function> after,
707+
v8::Local<v8::Function> resolve);
708+
704709
inline v8::Local<v8::String> provider_string(int idx);
705710

706711
inline void no_force_checks();
@@ -711,6 +716,9 @@ class AsyncHooks : public MemoryRetainer {
711716
inline bool pop_async_context(double async_id);
712717
inline void clear_async_id_stack(); // Used in fatal exceptions.
713718

719+
inline void AddContext(v8::Local<v8::Context> ctx);
720+
inline void RemoveContext(v8::Local<v8::Context> ctx);
721+
714722
AsyncHooks(const AsyncHooks&) = delete;
715723
AsyncHooks& operator=(const AsyncHooks&) = delete;
716724
AsyncHooks(AsyncHooks&&) = delete;
@@ -770,6 +778,10 @@ class AsyncHooks : public MemoryRetainer {
770778

771779
// Non-empty during deserialization
772780
const SerializeInfo* info_ = nullptr;
781+
782+
std::vector<v8::Global<v8::Context>> contexts_;
783+
784+
std::array<v8::Global<v8::Function>, 4> js_promise_hooks_;
773785
};
774786

775787
class ImmediateInfo : public MemoryRetainer {

‎src/node_contextify.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,11 @@ ContextifyContext::ContextifyContext(
127127

128128
ContextifyContext::~ContextifyContext() {
129129
env()->RemoveCleanupHook(CleanupHook, this);
130+
Isolate* isolate = env()->isolate();
131+
HandleScope scope(isolate);
132+
133+
env()->async_hooks()
134+
->RemoveContext(PersistentToLocal::Weak(isolate, context_));
130135
}
131136

132137

‎test/parallel/test-async-local-storage-contexts.js‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
'use strict';
2+
3+
require('../common');
4+
const assert = require('assert');
5+
const vm = require('vm');
6+
const { AsyncLocalStorage } = require('async_hooks');
7+
8+
// Regression test for https://github.com/nodejs/node/issues/38781
9+
10+
const context = vm.createContext({
11+
AsyncLocalStorage,
12+
assert
13+
});
14+
15+
vm.runInContext(`
16+
const storage = new AsyncLocalStorage()
17+
async function test() {
18+
return storage.run({ test: 'vm' }, async () => {
19+
assert.strictEqual(storage.getStore().test, 'vm');
20+
await 42;
21+
assert.strictEqual(storage.getStore().test, 'vm');
22+
});
23+
}
24+
test()
25+
`, context);
26+
27+
const storage = new AsyncLocalStorage();
28+
async function test() {
29+
return storage.run({ test: 'main context' }, async () => {
30+
assert.strictEqual(storage.getStore().test, 'main context');
31+
await 42;
32+
assert.strictEqual(storage.getStore().test, 'main context');
33+
});
34+
}
35+
test();

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL