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

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

/ node Public

Commit e404841

Browse files
bengl
authored and
MylesBorins
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 Backport-PR-URL: #38577 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 f3563d3 commit e404841

6 files changed

Lines changed: 106 additions & 2 deletions

File tree

‎src/async_wrap.cc‎

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

494494
static void SetPromiseHooks(const FunctionCallbackInfo<Value>& args) {
495495
Environment* env = Environment::GetCurrent(args);
496-
Local<Context> ctx = env->context();
497-
ctx->SetPromiseHooks(
496+
497+
env->async_hooks()->SetJSPromiseHooks(
498498
args[0]->IsFunction() ? args[0].As<Function>() : Local<Function>(),
499499
args[1]->IsFunction() ? args[1].As<Function>() : Local<Function>(),
500500
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
@@ -118,6 +118,20 @@ v8::Local<v8::Object> AsyncHooks::native_execution_async_resource(size_t i) {
118118
return PersistentToLocal::Strong(native_execution_async_resources_[i]);
119119
}
120120

121+
inline void AsyncHooks::SetJSPromiseHooks(v8::Local<v8::Function> init,
122+
v8::Local<v8::Function> before,
123+
v8::Local<v8::Function> after,
124+
v8::Local<v8::Function> resolve) {
125+
js_promise_hooks_[0].Reset(env()->isolate(), init);
126+
js_promise_hooks_[1].Reset(env()->isolate(), before);
127+
js_promise_hooks_[2].Reset(env()->isolate(), after);
128+
js_promise_hooks_[3].Reset(env()->isolate(), resolve);
129+
for (auto it = contexts_.begin(); it != contexts_.end(); it++) {
130+
PersistentToLocal::Weak(env()->isolate(), *it)
131+
->SetPromiseHooks(init, before, after, resolve);
132+
}
133+
}
134+
121135
inline v8::Local<v8::String> AsyncHooks::provider_string(int idx) {
122136
return env()->isolate_data()->async_wrap_provider(idx);
123137
}
@@ -240,6 +254,41 @@ void AsyncHooks::clear_async_id_stack() {
240254
fields_[kStackLength] = 0;
241255
}
242256

257+
inline void AsyncHooks::AddContext(v8::Local<v8::Context> ctx) {
258+
ctx->SetPromiseHooks(
259+
js_promise_hooks_[0].IsEmpty() ?
260+
v8::Local<v8::Function>() :
261+
PersistentToLocal::Strong(js_promise_hooks_[0]),
262+
js_promise_hooks_[1].IsEmpty() ?
263+
v8::Local<v8::Function>() :
264+
PersistentToLocal::Strong(js_promise_hooks_[1]),
265+
js_promise_hooks_[2].IsEmpty() ?
266+
v8::Local<v8::Function>() :
267+
PersistentToLocal::Strong(js_promise_hooks_[2]),
268+
js_promise_hooks_[3].IsEmpty() ?
269+
v8::Local<v8::Function>() :
270+
PersistentToLocal::Strong(js_promise_hooks_[3]));
271+
272+
size_t id = contexts_.size();
273+
contexts_.resize(id + 1);
274+
contexts_[id].Reset(env()->isolate(), ctx);
275+
contexts_[id].SetWeak();
276+
}
277+
278+
inline void AsyncHooks::RemoveContext(v8::Local<v8::Context> ctx) {
279+
v8::Isolate* isolate = env()->isolate();
280+
v8::HandleScope handle_scope(isolate);
281+
for (auto it = contexts_.begin(); it != contexts_.end(); it++) {
282+
v8::Local<v8::Context> saved_context =
283+
PersistentToLocal::Weak(env()->isolate(), *it);
284+
if (saved_context == ctx) {
285+
it->Reset();
286+
contexts_.erase(it);
287+
break;
288+
}
289+
}
290+
}
291+
243292
// The DefaultTriggerAsyncIdScope(AsyncWrap*) constructor is defined in
244293
// async_wrap-inl.h to avoid a circular dependency.
245294

@@ -333,6 +382,8 @@ inline void Environment::AssignToContext(v8::Local<v8::Context> context,
333382
#if HAVE_INSPECTOR
334383
inspector_agent()->ContextCreated(context, info);
335384
#endif // HAVE_INSPECTOR
385+
386+
this->async_hooks()->AddContext(context);
336387
}
337388

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

‎src/env.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -997,6 +997,7 @@ void AsyncHooks::MemoryInfo(MemoryTracker* tracker) const {
997997
tracker->TrackField("async_ids_stack", async_ids_stack_);
998998
tracker->TrackField("fields", fields_);
999999
tracker->TrackField("async_id_fields", async_id_fields_);
1000+
tracker->TrackField("js_promise_hooks", js_promise_hooks_);
10001001
}
10011002

10021003
void AsyncHooks::grow_async_ids_stack() {

‎src/env.h‎

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

651+
inline void SetJSPromiseHooks(v8::Local<v8::Function> init,
652+
v8::Local<v8::Function> before,
653+
v8::Local<v8::Function> after,
654+
v8::Local<v8::Function> resolve);
655+
651656
inline v8::Local<v8::String> provider_string(int idx);
652657

653658
inline void no_force_checks();
@@ -658,6 +663,9 @@ class AsyncHooks : public MemoryRetainer {
658663
inline bool pop_async_context(double async_id);
659664
inline void clear_async_id_stack(); // Used in fatal exceptions.
660665

666+
inline void AddContext(v8::Local<v8::Context> ctx);
667+
inline void RemoveContext(v8::Local<v8::Context> ctx);
668+
661669
AsyncHooks(const AsyncHooks&) = delete;
662670
AsyncHooks& operator=(const AsyncHooks&) = delete;
663671
AsyncHooks(AsyncHooks&&) = delete;
@@ -701,6 +709,10 @@ class AsyncHooks : public MemoryRetainer {
701709

702710
v8::Global<v8::Array> js_execution_async_resources_;
703711
std::vector<v8::Global<v8::Object>> native_execution_async_resources_;
712+
713+
std::vector<v8::Global<v8::Context>> contexts_;
714+
715+
std::array<v8::Global<v8::Function>, 4> js_promise_hooks_;
704716
};
705717

706718
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