-
-
Notifications
You must be signed in to change notification settings - Fork 32.8k
src: set PromiseHooks by Environment #38821
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7986a9b
bb55976
0f4342b
d1667ad
d3ec54a
add2360
a968969
5ea90f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -701,6 +701,11 @@ class AsyncHooks : public MemoryRetainer { | |
// The `js_execution_async_resources` array contains the value in that case. | ||
inline v8::Local<v8::Object> native_execution_async_resource(size_t index); | ||
|
||
inline void SetJSPromiseHooks(v8::Local<v8::Function> init, | ||
v8::Local<v8::Function> before, | ||
v8::Local<v8::Function> after, | ||
v8::Local<v8::Function> resolve); | ||
|
||
inline v8::Local<v8::String> provider_string(int idx); | ||
|
||
inline void no_force_checks(); | ||
|
@@ -711,6 +716,9 @@ class AsyncHooks : public MemoryRetainer { | |
inline bool pop_async_context(double async_id); | ||
inline void clear_async_id_stack(); // Used in fatal exceptions. | ||
|
||
inline void AddContext(v8::Local<v8::Context> ctx); | ||
inline void RemoveContext(v8::Local<v8::Context> ctx); | ||
|
||
AsyncHooks(const AsyncHooks&) = delete; | ||
AsyncHooks& operator=(const AsyncHooks&) = delete; | ||
AsyncHooks(AsyncHooks&&) = delete; | ||
|
@@ -770,6 +778,10 @@ class AsyncHooks : public MemoryRetainer { | |
|
||
// Non-empty during deserialization | ||
const SerializeInfo* info_ = nullptr; | ||
|
||
std::vector<v8::Global<v8::Context>> contexts_; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally we should implement snapshot support for these, but since they aren't used by default in the bootstrap, I think we could just CHECK that these are empty in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @joyeecheung There will be one (weak ref'd) context in the vector at snapshot time (the main context). Do you mean the promise hook functions? e.g. diff --git a/src/env.cc b/src/env.cc
index 042b7243ac..f4d789710e 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -1156,6 +1156,11 @@ AsyncHooks::SerializeInfo AsyncHooks::Serialize(Local<Context> context,
context,
native_execution_async_resources_[i].Get(context->GetIsolate()));
}
+ CHECK_EQ(contexts_.size(), 1);
+ CHECK(js_promise_hooks_[0].IsEmpty());
+ CHECK(js_promise_hooks_[1].IsEmpty());
+ CHECK(js_promise_hooks_[2].IsEmpty());
+ CHECK(js_promise_hooks_[3].IsEmpty());
return info;
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, yes, that looks about right (maybe for additional safety, you could check that one context is indeed the main context) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright, done in latest fixup commit 👍 |
||
|
||
std::array<v8::Global<v8::Function>, 4> js_promise_hooks_; | ||
}; | ||
|
||
class ImmediateInfo : public MemoryRetainer { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
'use strict'; | ||
|
||
require('../common'); | ||
const assert = require('assert'); | ||
const vm = require('vm'); | ||
const { AsyncLocalStorage } = require('async_hooks'); | ||
|
||
// Regression test for https://github.com/nodejs/node/issues/38781 | ||
|
||
const context = vm.createContext({ | ||
AsyncLocalStorage, | ||
assert | ||
}); | ||
|
||
vm.runInContext(` | ||
const storage = new AsyncLocalStorage() | ||
async function test() { | ||
return storage.run({ test: 'vm' }, async () => { | ||
assert.strictEqual(storage.getStore().test, 'vm'); | ||
await 42; | ||
assert.strictEqual(storage.getStore().test, 'vm'); | ||
}); | ||
} | ||
test() | ||
`, context); | ||
|
||
const storage = new AsyncLocalStorage(); | ||
async function test() { | ||
return storage.run({ test: 'main context' }, async () => { | ||
assert.strictEqual(storage.getStore().test, 'main context'); | ||
await 42; | ||
assert.strictEqual(storage.getStore().test, 'main context'); | ||
}); | ||
} | ||
test(); |
Uh oh!
There was an error while loading. Please reload this page.