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

async_hooks: fix id assignment in fast-path promise hook · nodejs/node@e9aebc3 · GitHub

/ node Public

Commit e9aebc3

Browse files
puzpuzpuz
authored and
codebytere
committed
async_hooks: fix id assignment in fast-path promise hook
Native side of fast-path promise hook was not calling JS fastPromiseHook function when there were no async ids previously assigned to the promise. Because of that already created promises could not get id assigned in situations when an async hook without a before listener function is enabled after their creation. As the result executionAsyncId could return wrong id when called within promise's .then(). Refs: #34512 PR-URL: #34548 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
1 parent bd55236 commit e9aebc3

2 files changed

Lines changed: 69 additions & 38 deletions

File tree

‎src/async_wrap.cc‎

Lines changed: 44 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,12 @@ using v8::Global;
3838
using v8::HandleScope;
3939
using v8::Integer;
4040
using v8::Isolate;
41+
using v8::Just;
4142
using v8::Local;
43+
using v8::Maybe;
4244
using v8::MaybeLocal;
4345
using v8::Name;
46+
using v8::Nothing;
4447
using v8::Number;
4548
using v8::Object;
4649
using v8::ObjectTemplate;
@@ -188,6 +191,21 @@ void AsyncWrap::EmitAfter(Environment* env, double async_id) {
188191
env->async_hooks_after_function());
189192
}
190193

194+
// TODO(addaleax): Remove once we're on C++17.
195+
constexpr double AsyncWrap::kInvalidAsyncId;
196+
197+
static Maybe<double> GetAssignedPromiseAsyncId(Environment* env,
198+
Local<Promise> promise,
199+
Local<Value> id_symbol) {
200+
Local<Value> maybe_async_id;
201+
if (!promise->Get(env->context(), id_symbol).ToLocal(&maybe_async_id)) {
202+
return Nothing<double>();
203+
}
204+
return maybe_async_id->IsNumber()
205+
? maybe_async_id->NumberValue(env->context())
206+
: Just(AsyncWrap::kInvalidAsyncId);
207+
}
208+
191209
class PromiseWrap : public AsyncWrap {
192210
public:
193211
PromiseWrap(Environment* env, Local<Object> object, bool silent)
@@ -230,18 +248,17 @@ PromiseWrap* PromiseWrap::New(Environment* env,
230248

231249
// Skip for init events
232250
if (silent) {
233-
Local<Value> maybe_async_id = promise
234-
->Get(context, env->async_id_symbol())
235-
.ToLocalChecked();
236-
237-
Local<Value> maybe_trigger_async_id = promise
238-
->Get(context, env->trigger_async_id_symbol())
239-
.ToLocalChecked();
240-
241-
if (maybe_async_id->IsNumber() && maybe_trigger_async_id->IsNumber()) {
242-
double async_id = maybe_async_id.As<Number>()->Value();
243-
double trigger_async_id = maybe_trigger_async_id.As<Number>()->Value();
244-
return new PromiseWrap(env, obj, async_id, trigger_async_id);
251+
double async_id;
252+
double trigger_async_id;
253+
if (!GetAssignedPromiseAsyncId(env, promise, env->async_id_symbol())
254+
.To(&async_id)) return nullptr;
255+
if (!GetAssignedPromiseAsyncId(env, promise, env->trigger_async_id_symbol())
256+
.To(&trigger_async_id)) return nullptr;
257+
258+
if (async_id != AsyncWrap::kInvalidAsyncId &&
259+
trigger_async_id != AsyncWrap::kInvalidAsyncId) {
260+
return new PromiseWrap(
261+
env, obj, async_id, trigger_async_id);
245262
}
246263
}
247264

@@ -320,46 +337,35 @@ static void FastPromiseHook(PromiseHookType type, Local<Promise> promise,
320337

321338
if (type == PromiseHookType::kBefore &&
322339
env->async_hooks()->fields()[AsyncHooks::kBefore] == 0) {
323-
Local<Value> maybe_async_id;
324-
if (!promise->Get(context, env->async_id_symbol())
325-
.ToLocal(&maybe_async_id)) {
326-
return;
327-
}
328-
329-
Local<Value> maybe_trigger_async_id;
330-
if (!promise->Get(context, env->trigger_async_id_symbol())
331-
.ToLocal(&maybe_trigger_async_id)) {
332-
return;
333-
}
334-
335-
if (maybe_async_id->IsNumber() && maybe_trigger_async_id->IsNumber()) {
336-
double async_id = maybe_async_id.As<Number>()->Value();
337-
double trigger_async_id = maybe_trigger_async_id.As<Number>()->Value();
340+
double async_id;
341+
double trigger_async_id;
342+
if (!GetAssignedPromiseAsyncId(env, promise, env->async_id_symbol())
343+
.To(&async_id)) return;
344+
if (!GetAssignedPromiseAsyncId(env, promise, env->trigger_async_id_symbol())
345+
.To(&trigger_async_id)) return;
346+
347+
if (async_id != AsyncWrap::kInvalidAsyncId &&
348+
trigger_async_id != AsyncWrap::kInvalidAsyncId) {
338349
env->async_hooks()->push_async_context(
339350
async_id, trigger_async_id, promise);
351+
return;
340352
}
341-
342-
return;
343353
}
344354

345355
if (type == PromiseHookType::kAfter &&
346356
env->async_hooks()->fields()[AsyncHooks::kAfter] == 0) {
347-
Local<Value> maybe_async_id;
348-
if (!promise->Get(context, env->async_id_symbol())
349-
.ToLocal(&maybe_async_id)) {
350-
return;
351-
}
357+
double async_id;
358+
if (!GetAssignedPromiseAsyncId(env, promise, env->async_id_symbol())
359+
.To(&async_id)) return;
352360

353-
if (maybe_async_id->IsNumber()) {
354-
double async_id = maybe_async_id.As<Number>()->Value();
361+
if (async_id != AsyncWrap::kInvalidAsyncId) {
355362
if (env->execution_async_id() == async_id) {
356363
// This condition might not be true if async_hooks was enabled during
357364
// the promise callback execution.
358365
env->async_hooks()->pop_async_context(async_id);
359366
}
367+
return;
360368
}
361-
362-
return;
363369
}
364370

365371
if (type == PromiseHookType::kResolve &&

‎test/parallel/test-async-hooks-enable-before-promise-resolve.js‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const async_hooks = require('async_hooks');
5+
6+
// This test ensures that fast-path PromiseHook assigns async ids
7+
// to already created promises when the native hook function is
8+
// triggered on before event.
9+
10+
let initialAsyncId;
11+
const promise = new Promise((resolve) => {
12+
setTimeout(() => {
13+
initialAsyncId = async_hooks.executionAsyncId();
14+
async_hooks.createHook({
15+
after: common.mustCall(() => {}, 2)
16+
}).enable();
17+
resolve();
18+
}, 0);
19+
});
20+
21+
promise.then(common.mustCall(() => {
22+
const id = async_hooks.executionAsyncId();
23+
assert.notStrictEqual(id, initialAsyncId);
24+
assert.ok(id > 0);
25+
}));

0 commit comments

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