Skip to content

Commit 9b75102

Browse files
committed
async_hooks: move PromiseHook handler to JS
This avoids the need to wrap every promise in an AsyncWrap and also makes it easier to skip the machinery to track destroy events when there's no destroy listener.
1 parent 5a4f24e commit 9b75102

File tree

6 files changed

+148
-113
lines changed

6 files changed

+148
-113
lines changed

TODO.md

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# C++
2+
3+
* [ ] Move async_hooks-related methods from Environment to AsyncHooks.
4+
5+
```cpp
6+
// The necessary API for async_hooks.
7+
inline double new_async_id();
8+
inline double execution_async_id();
9+
inline double trigger_async_id();
10+
inline double get_default_trigger_async_id();
11+
```

lib/internal/async_hooks.js

+79-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const {
44
Error,
55
FunctionPrototypeBind,
66
ObjectDefineProperty,
7+
Promise,
78
Symbol,
89
} = primordials;
910

@@ -243,13 +244,90 @@ function restoreActiveHooks() {
243244
active_hooks.tmp_fields = null;
244245
}
245246

247+
const promiseWrap = Symbol('promiseWrap');
248+
const destroyedSymbol = Symbol('destroyed');
249+
250+
class PromiseWrap {
251+
constructor(promise, parent_wrap, silent) {
252+
promise[promiseWrap] = this;
253+
this.isChainedPromise = !!parent_wrap;
254+
this.promise = promise;
255+
this[destroyedSymbol] = undefined;
256+
257+
const asyncId = getOrSetAsyncId(this);
258+
259+
if (typeof parent_wrap !== 'undefined') {
260+
this.triggerAsyncId = getOrSetAsyncId(parent_wrap);
261+
} else {
262+
this.triggerAsyncId = getDefaultTriggerAsyncId();
263+
}
264+
265+
if (!silent && initHooksExist()) {
266+
emitInitScript(asyncId, 'PROMISE', this.triggerAsyncId, this);
267+
268+
if (destroyHooksExist()) {
269+
const destroyed = { destroyed: false };
270+
this[destroyedSymbol] = destroyed;
271+
registerDestroyHook(this, asyncId, destroyed);
272+
}
273+
}
274+
}
275+
276+
before() {
277+
emitBeforeScript(getOrSetAsyncId(this), this.triggerAsyncId, this);
278+
}
279+
280+
after() {
281+
emitAfterScript(getOrSetAsyncId(this));
282+
}
283+
284+
promiseResolve() {
285+
emitPromiseResolveNative(getOrSetAsyncId(this));
286+
}
287+
288+
static from(promise) {
289+
return promise && promise[promiseWrap];
290+
}
291+
}
292+
293+
function promiseHook(type, promise, parent) {
294+
let wrap = PromiseWrap.from(promise);
295+
if (type === 'init' || !wrap) {
296+
const silent = type !== 'init';
297+
if (parent instanceof Promise) {
298+
let parentWrap = PromiseWrap.from(parent);
299+
if (!parentWrap) {
300+
parentWrap = new PromiseWrap(parent, null, true);
301+
if (!parentWrap) return;
302+
}
303+
304+
wrap = new PromiseWrap(promise, parentWrap, silent);
305+
} else {
306+
wrap = new PromiseWrap(promise, undefined, silent);
307+
}
308+
}
309+
310+
if (!wrap) return;
311+
312+
switch (type) {
313+
case 'before':
314+
wrap.before();
315+
break;
316+
case 'after':
317+
wrap.after();
318+
break;
319+
case 'promiseResolve':
320+
wrap.promiseResolve();
321+
break;
322+
}
323+
}
246324

247325
let wantPromiseHook = false;
248326
function enableHooks() {
249327
async_hook_fields[kCheck] += 1;
250328

251329
wantPromiseHook = true;
252-
enablePromiseHook();
330+
enablePromiseHook(promiseHook);
253331
}
254332

255333
function disableHooks() {

src/async_wrap.cc

+15-111
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,9 @@ using v8::Local;
4242
using v8::MaybeLocal;
4343
using v8::Number;
4444
using v8::Object;
45-
using v8::ObjectTemplate;
4645
using v8::Promise;
4746
using v8::PromiseHookType;
4847
using v8::PropertyAttribute;
49-
using v8::PropertyCallbackInfo;
5048
using v8::ReadOnly;
5149
using v8::String;
5250
using v8::Uint32;
@@ -173,59 +171,6 @@ void AsyncWrap::EmitAfter(Environment* env, double async_id) {
173171
env->async_hooks_after_function());
174172
}
175173

176-
class PromiseWrap : public AsyncWrap {
177-
public:
178-
enum InternalFields {
179-
kIsChainedPromiseField = AsyncWrap::kInternalFieldCount,
180-
kInternalFieldCount
181-
};
182-
PromiseWrap(Environment* env, Local<Object> object, bool silent)
183-
: AsyncWrap(env, object, PROVIDER_PROMISE, kInvalidAsyncId, silent) {
184-
MakeWeak();
185-
}
186-
187-
SET_NO_MEMORY_INFO()
188-
SET_MEMORY_INFO_NAME(PromiseWrap)
189-
SET_SELF_SIZE(PromiseWrap)
190-
191-
static PromiseWrap* New(Environment* env,
192-
Local<Promise> promise,
193-
PromiseWrap* parent_wrap,
194-
bool silent);
195-
static void getIsChainedPromise(Local<String> property,
196-
const PropertyCallbackInfo<Value>& info);
197-
};
198-
199-
PromiseWrap* PromiseWrap::New(Environment* env,
200-
Local<Promise> promise,
201-
PromiseWrap* parent_wrap,
202-
bool silent) {
203-
Local<Object> obj;
204-
if (!env->promise_wrap_template()->NewInstance(env->context()).ToLocal(&obj))
205-
return nullptr;
206-
obj->SetInternalField(PromiseWrap::kIsChainedPromiseField,
207-
parent_wrap != nullptr ? v8::True(env->isolate())
208-
: v8::False(env->isolate()));
209-
CHECK_NULL(promise->GetAlignedPointerFromInternalField(0));
210-
promise->SetInternalField(0, obj);
211-
return new PromiseWrap(env, obj, silent);
212-
}
213-
214-
void PromiseWrap::getIsChainedPromise(Local<String> property,
215-
const PropertyCallbackInfo<Value>& info) {
216-
info.GetReturnValue().Set(
217-
info.Holder()->GetInternalField(PromiseWrap::kIsChainedPromiseField));
218-
}
219-
220-
static PromiseWrap* extractPromiseWrap(Local<Promise> promise) {
221-
// This check is imperfect. If the internal field is set, it should
222-
// be an object. If it's not, we just ignore it. Ideally v8 would
223-
// have had GetInternalField returning a MaybeLocal but this works
224-
// for now.
225-
Local<Value> obj = promise->GetInternalField(0);
226-
return obj->IsObject() ? Unwrap<PromiseWrap>(obj.As<Object>()) : nullptr;
227-
}
228-
229174
static void PromiseHook(PromiseHookType type, Local<Promise> promise,
230175
Local<Value> parent) {
231176
Local<Context> context = promise->CreationContext();
@@ -235,50 +180,14 @@ static void PromiseHook(PromiseHookType type, Local<Promise> promise,
235180
TraceEventScope trace_scope(TRACING_CATEGORY_NODE1(environment),
236181
"EnvPromiseHook", env);
237182

238-
PromiseWrap* wrap = extractPromiseWrap(promise);
239-
if (type == PromiseHookType::kInit || wrap == nullptr) {
240-
bool silent = type != PromiseHookType::kInit;
241-
242-
// set parent promise's async Id as this promise's triggerAsyncId
243-
if (parent->IsPromise()) {
244-
// parent promise exists, current promise
245-
// is a chained promise, so we set parent promise's id as
246-
// current promise's triggerAsyncId
247-
Local<Promise> parent_promise = parent.As<Promise>();
248-
PromiseWrap* parent_wrap = extractPromiseWrap(parent_promise);
249-
if (parent_wrap == nullptr) {
250-
parent_wrap = PromiseWrap::New(env, parent_promise, nullptr, true);
251-
if (parent_wrap == nullptr) return;
252-
}
253-
254-
AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(parent_wrap);
255-
wrap = PromiseWrap::New(env, promise, parent_wrap, silent);
256-
} else {
257-
wrap = PromiseWrap::New(env, promise, nullptr, silent);
258-
}
259-
}
183+
Local<Value> argv[] = {
184+
env->isolate_data()->promise_hook_type(static_cast<int>(type)),
185+
promise,
186+
parent
187+
};
260188

261-
if (wrap == nullptr) return;
262-
263-
if (type == PromiseHookType::kBefore) {
264-
env->async_hooks()->push_async_context(wrap->get_async_id(),
265-
wrap->get_trigger_async_id(), wrap->object());
266-
wrap->EmitTraceEventBefore();
267-
AsyncWrap::EmitBefore(wrap->env(), wrap->get_async_id());
268-
} else if (type == PromiseHookType::kAfter) {
269-
wrap->EmitTraceEventAfter(wrap->provider_type(), wrap->get_async_id());
270-
AsyncWrap::EmitAfter(wrap->env(), wrap->get_async_id());
271-
if (env->execution_async_id() == wrap->get_async_id()) {
272-
// This condition might not be true if async_hooks was enabled during
273-
// the promise callback execution.
274-
// Popping it off the stack can be skipped in that case, because it is
275-
// known that it would correspond to exactly one call with
276-
// PromiseHookType::kBefore that was not witnessed by the PromiseHook.
277-
env->async_hooks()->pop_async_context(wrap->get_async_id());
278-
}
279-
} else if (type == PromiseHookType::kResolve) {
280-
AsyncWrap::EmitPromiseResolve(wrap->env(), wrap->get_async_id());
281-
}
189+
Local<Function> promise_hook = env->promise_hook_handler();
190+
USE(promise_hook->Call(context, Undefined(env->isolate()), 3, argv));
282191
}
283192

284193

@@ -310,23 +219,18 @@ static void SetupHooks(const FunctionCallbackInfo<Value>& args) {
310219
SET_HOOK_FN(destroy);
311220
SET_HOOK_FN(promise_resolve);
312221
#undef SET_HOOK_FN
222+
}
313223

314-
{
315-
Local<FunctionTemplate> ctor =
316-
FunctionTemplate::New(env->isolate());
317-
ctor->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "PromiseWrap"));
318-
Local<ObjectTemplate> promise_wrap_template = ctor->InstanceTemplate();
319-
promise_wrap_template->SetInternalFieldCount(
320-
PromiseWrap::kInternalFieldCount);
321-
promise_wrap_template->SetAccessor(
322-
FIXED_ONE_BYTE_STRING(env->isolate(), "isChainedPromise"),
323-
PromiseWrap::getIsChainedPromise);
324-
env->set_promise_wrap_template(promise_wrap_template);
224+
static void EnablePromiseHook(const FunctionCallbackInfo<Value>& args) {
225+
Environment* env = Environment::GetCurrent(args);
226+
227+
if (!args[0]->IsFunction()) {
228+
return node::THROW_ERR_INVALID_ARG_TYPE(
229+
env, "handler must be a function");
325230
}
326-
}
327231

232+
env->set_promise_hook_handler(args[0].As<Function>());
328233

329-
static void EnablePromiseHook(const FunctionCallbackInfo<Value>& args) {
330234
args.GetIsolate()->SetPromiseHook(PromiseHook);
331235
}
332236

src/env-inl.h

+4
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@ inline v8::Local<v8::String> IsolateData::async_wrap_provider(int index) const {
7777
return async_wrap_providers_[index].Get(isolate_);
7878
}
7979

80+
inline v8::Local<v8::String> IsolateData::promise_hook_type(int index) const {
81+
return promise_hook_types_[index].Get(isolate_);
82+
}
83+
8084
inline AsyncHooks::AsyncHooks()
8185
: async_ids_stack_(env()->isolate(), 16 * 2),
8286
fields_(env()->isolate(), kFieldsCount),

src/env.cc

+33
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ std::vector<size_t> IsolateData::Serialize(SnapshotCreator* creator) {
8282
for (size_t i = 0; i < AsyncWrap::PROVIDERS_LENGTH; i++)
8383
indexes.push_back(creator->AddData(async_wrap_provider(i)));
8484

85+
for (size_t i = 0; i < 4; i++)
86+
indexes.push_back(creator->AddData(promise_hook_type(i)));
87+
8588
return indexes;
8689
}
8790

@@ -117,6 +120,15 @@ void IsolateData::DeserializeProperties(const std::vector<size_t>* indexes) {
117120
}
118121
async_wrap_providers_[j].Set(isolate_, field.ToLocalChecked());
119122
}
123+
124+
for (size_t j = 0; j < 4; j++) {
125+
MaybeLocal<String> field =
126+
isolate_->GetDataFromSnapshotOnce<String>((*indexes)[i++]);
127+
if (field.IsEmpty()) {
128+
fprintf(stderr, "Failed to deserialize PromiseHook type %zu\n", j);
129+
}
130+
promise_hook_types_[j].Set(isolate_, field.ToLocalChecked());
131+
}
120132
}
121133

122134
void IsolateData::CreateProperties() {
@@ -181,6 +193,26 @@ void IsolateData::CreateProperties() {
181193
sizeof(#Provider) - 1).ToLocalChecked());
182194
NODE_ASYNC_PROVIDER_TYPES(V)
183195
#undef V
196+
197+
// Create all the hook type strings that will be passed to JS. Place them in
198+
// an array so the array index matches the type id offset. This way the
199+
// strings can be retrieved quickly.
200+
#define PROMISE_HOOK_TYPES(V) \
201+
V(0, init) \
202+
V(1, promiseResolve) \
203+
V(2, before) \
204+
V(3, after)
205+
#define V(index, name) \
206+
promise_hook_types_[index].Set( \
207+
isolate_, \
208+
v8::String::NewFromOneByte( \
209+
isolate_, \
210+
reinterpret_cast<const uint8_t*>(#name), \
211+
v8::NewStringType::kInternalized, \
212+
sizeof(#name) - 1).ToLocalChecked());
213+
PROMISE_HOOK_TYPES(V)
214+
#undef V
215+
#undef PROMISE_HOOK_TYPES
184216
}
185217

186218
IsolateData::IsolateData(Isolate* isolate,
@@ -219,6 +251,7 @@ void IsolateData::MemoryInfo(MemoryTracker* tracker) const {
219251
#undef V
220252

221253
tracker->TrackField("async_wrap_providers", async_wrap_providers_);
254+
tracker->TrackField("promise_hook_types", promise_hook_types_);
222255

223256
if (node_allocator_ != nullptr) {
224257
tracker->TrackFieldWithSize(

src/env.h

+6-1
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,6 @@ constexpr size_t kFsStatsBufferLength =
408408
V(libuv_stream_wrap_ctor_template, v8::FunctionTemplate) \
409409
V(message_port_constructor_template, v8::FunctionTemplate) \
410410
V(pipe_constructor_template, v8::FunctionTemplate) \
411-
V(promise_wrap_template, v8::ObjectTemplate) \
412411
V(sab_lifetimepartner_constructor_template, v8::FunctionTemplate) \
413412
V(script_context_constructor_template, v8::FunctionTemplate) \
414413
V(secure_context_constructor_template, v8::FunctionTemplate) \
@@ -458,6 +457,7 @@ constexpr size_t kFsStatsBufferLength =
458457
V(prepare_stack_trace_callback, v8::Function) \
459458
V(process_object, v8::Object) \
460459
V(primordials, v8::Object) \
460+
V(promise_hook_handler, v8::Function) \
461461
V(promise_reject_callback, v8::Function) \
462462
V(script_data_constructor_function, v8::Function) \
463463
V(source_map_cache_getter, v8::Function) \
@@ -507,6 +507,7 @@ class IsolateData : public MemoryRetainer {
507507
#undef VS
508508
#undef VP
509509
inline v8::Local<v8::String> async_wrap_provider(int index) const;
510+
inline v8::Local<v8::String> promise_hook_type(int index) const;
510511

511512
std::unordered_map<const char*, v8::Eternal<v8::String>> static_str_map;
512513

@@ -536,6 +537,10 @@ class IsolateData : public MemoryRetainer {
536537
std::array<v8::Eternal<v8::String>, AsyncWrap::PROVIDERS_LENGTH>
537538
async_wrap_providers_;
538539

540+
// Keep a list of all Persistent strings used for PromiseHook types.
541+
std::array<v8::Eternal<v8::String>, 4>
542+
promise_hook_types_;
543+
539544
v8::Isolate* const isolate_;
540545
uv_loop_t* const event_loop_;
541546
v8::ArrayBuffer::Allocator* const allocator_;

0 commit comments

Comments
 (0)