From aba0cf33ecf97fe690f0b5de5c3c103076e23b3a Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Fri, 5 Jul 2019 18:01:56 -0700 Subject: [PATCH] inspector: do not change async call stack depth if the worker is done Fixes: https://github.com/nodejs/node/issues/28528 PR-URL: https://github.com/nodejs/node/pull/28613 Reviewed-By: Aleksei Koziatinskii Reviewed-By: James M Snell --- src/inspector_agent.cc | 5 ++ .../test-inspector-async-hook-after-done.js | 60 +++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 test/parallel/test-inspector-async-hook-after-done.js diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 6a62f79e15843b..4a4a8eb4be0754 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -483,6 +483,11 @@ class NodeInspectorClient : public V8InspectorClient { } void maxAsyncCallStackDepthChanged(int depth) override { + if (waiting_for_sessions_disconnect_) { + // V8 isolate is mostly done and is only letting Inspector protocol + // clients gather data. + return; + } if (auto agent = env_->inspector_agent()) { if (depth == 0) { agent->DisableAsyncHook(); diff --git a/test/parallel/test-inspector-async-hook-after-done.js b/test/parallel/test-inspector-async-hook-after-done.js new file mode 100644 index 00000000000000..10916acd0f8647 --- /dev/null +++ b/test/parallel/test-inspector-async-hook-after-done.js @@ -0,0 +1,60 @@ +'use strict'; + +const common = require('../common'); + +common.skipIfInspectorDisabled(); + +const assert = require('assert'); +const { Worker } = require('worker_threads'); +const { Session } = require('inspector'); + +const session = new Session(); + +let done = false; + +session.connect(); + +session.on('NodeWorker.attachedToWorker', ({ params: { sessionId } }) => { + let id = 1; + function postToWorkerInspector(method, params) { + session.post('NodeWorker.sendMessageToWorker', { + sessionId, + message: JSON.stringify({ id: id++, method, params }) + }, () => console.log(`Message ${method} received the response`)); + } + + // Wait for the notification + function onMessageReceived({ params: { message } }) { + if (!message || + JSON.parse(message).method !== 'NodeRuntime.waitingForDisconnect') { + session.once('NodeWorker.receivedMessageFromWorker', onMessageReceived); + return; + } + // Force a call to node::inspector::Agent::ToggleAsyncHook by changing the + // async call stack depth + postToWorkerInspector('Debugger.setAsyncCallStackDepth', { maxDepth: 1 }); + // This is were the original crash happened + session.post('NodeWorker.detach', { sessionId }, () => { + done = true; + }); + } + + onMessageReceived({ params: { message: null } }); + // Enable the debugger, otherwise setAsyncCallStackDepth does nothing + postToWorkerInspector('Debugger.enable'); + // Start waiting for disconnect notification + postToWorkerInspector('NodeRuntime.notifyWhenWaitingForDisconnect', + { enabled: true }); + // start worker + postToWorkerInspector('Runtime.runIfWaitingForDebugger'); +}); + +session.post('NodeWorker.enable', { waitForDebuggerOnStart: true }, () => { + new Worker('console.log("Worker is done")', { eval: true }) + .once('exit', () => { + setTimeout(() => { + assert.strictEqual(done, true); + console.log('Test is done'); + }, 0); + }); +});