From 0fb6f709c9d36aa743fdc50a131900f3fa790f81 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 18 Nov 2017 16:37:07 +0100 Subject: [PATCH 1/2] inspector: no async tracking for promises `Promise` instances are already tracked by V8 itself. This fixes `sequential/test-inspector-async-stack-traces-promise-then` in debug mode (it previously crashed because our tracking and the V8 tracking were not properly nested). Ref: https://chromium-review.googlesource.com/c/v8/v8/+/707058 Fixes: https://github.com/nodejs/node/issues/17017 --- lib/internal/inspector_async_hook.js | 13 ++++++++++++- ...est-inspector-async-stack-traces-promise-then.js | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/internal/inspector_async_hook.js b/lib/internal/inspector_async_hook.js index 4ccf862697756d..6fc197e8b85f7f 100644 --- a/lib/internal/inspector_async_hook.js +++ b/lib/internal/inspector_async_hook.js @@ -16,22 +16,33 @@ const hook = createHook({ // in https://github.com/nodejs/node/pull/13870#discussion_r124515293, // this should be fine as long as we call asyncTaskCanceled() too. const recurring = true; - inspector.asyncTaskScheduled(type, asyncId, recurring); + if (type === 'PROMISE') + this.promises.add(asyncId); + else + inspector.asyncTaskScheduled(type, asyncId, recurring); }, before(asyncId) { + if (this.promises.has(asyncId)) + return; inspector.asyncTaskStarted(asyncId); }, after(asyncId) { + if (this.promises.has(asyncId)) + return; inspector.asyncTaskFinished(asyncId); }, destroy(asyncId) { + if (this.promises.has(asyncId)) + return this.promises.delete(asyncId); inspector.asyncTaskCanceled(asyncId); }, }); +hook.promises = new Set(); + function enable() { if (config.bits < 64) { // V8 Inspector stores task ids as (void*) pointers. diff --git a/test/sequential/test-inspector-async-stack-traces-promise-then.js b/test/sequential/test-inspector-async-stack-traces-promise-then.js index a321855b5ea613..9ed61d5ae13675 100644 --- a/test/sequential/test-inspector-async-stack-traces-promise-then.js +++ b/test/sequential/test-inspector-async-stack-traces-promise-then.js @@ -54,7 +54,7 @@ function debuggerPausedAt(msg, functionName, previousTickLocation) { `${Object.keys(msg.params)} contains "asyncStackTrace" property`); assert.strictEqual(msg.params.callFrames[0].functionName, functionName); - assert.strictEqual(msg.params.asyncStackTrace.description, 'PROMISE'); + assert.strictEqual(msg.params.asyncStackTrace.description, 'Promise.resolve'); const frameLocations = msg.params.asyncStackTrace.callFrames.map( (frame) => `${frame.functionName}:${frame.lineNumber}`); From d6a59913d36b3a972caabaafb612e7ce1f8098e6 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 18 Nov 2017 19:08:42 +0100 Subject: [PATCH 2/2] =?UTF-8?q?[squash]=20rename=20promises=20=E2=86=92=20?= =?UTF-8?q?promiseIds?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/internal/inspector_async_hook.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/internal/inspector_async_hook.js b/lib/internal/inspector_async_hook.js index 6fc197e8b85f7f..b190ff90b8ea2c 100644 --- a/lib/internal/inspector_async_hook.js +++ b/lib/internal/inspector_async_hook.js @@ -17,31 +17,31 @@ const hook = createHook({ // this should be fine as long as we call asyncTaskCanceled() too. const recurring = true; if (type === 'PROMISE') - this.promises.add(asyncId); + this.promiseIds.add(asyncId); else inspector.asyncTaskScheduled(type, asyncId, recurring); }, before(asyncId) { - if (this.promises.has(asyncId)) + if (this.promiseIds.has(asyncId)) return; inspector.asyncTaskStarted(asyncId); }, after(asyncId) { - if (this.promises.has(asyncId)) + if (this.promiseIds.has(asyncId)) return; inspector.asyncTaskFinished(asyncId); }, destroy(asyncId) { - if (this.promises.has(asyncId)) - return this.promises.delete(asyncId); + if (this.promiseIds.has(asyncId)) + return this.promiseIds.delete(asyncId); inspector.asyncTaskCanceled(asyncId); }, }); -hook.promises = new Set(); +hook.promiseIds = new Set(); function enable() { if (config.bits < 64) {