From 8cbf72e8317e5e90cd67028b7b6a94734100b6c6 Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Fri, 26 Jul 2019 16:38:15 -0700 Subject: [PATCH 1/5] inspector: report all workers Main thread (the one that WS endpoint connects to) should be able to report all workers. --- src/inspector/worker_inspector.h | 7 ++ src/inspector_agent.cc | 21 ++++-- .../test-inspector-workers-flat-list.js | 70 +++++++++++++++++++ 3 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-inspector-workers-flat-list.js diff --git a/src/inspector/worker_inspector.h b/src/inspector/worker_inspector.h index cf483ae49ebbe1..c0961496cec1bd 100644 --- a/src/inspector/worker_inspector.h +++ b/src/inspector/worker_inspector.h @@ -55,6 +55,13 @@ class ParentInspectorHandle { std::shared_ptr parent_thread, bool wait_for_connect); ~ParentInspectorHandle(); + std::unique_ptr NewParentInspectorHandle( + int thread_id, const std::string& url) { + return std::make_unique(thread_id, + url, + parent_thread_, + wait_); + } void WorkerStarted(std::shared_ptr worker_thread, bool waiting); bool WaitForConnect() { diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index a1cdc606fe8e62..f4c3dbfdfdc0fc 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -237,8 +237,10 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel, tracing_agent_ = std::make_unique(env, main_thread_); tracing_agent_->Wire(node_dispatcher_.get()); - worker_agent_ = std::make_unique(worker_manager); - worker_agent_->Wire(node_dispatcher_.get()); + if (worker_manager) { + worker_agent_ = std::make_unique(worker_manager); + worker_agent_->Wire(node_dispatcher_.get()); + } runtime_agent_ = std::make_unique(); runtime_agent_->Wire(node_dispatcher_.get()); } @@ -246,8 +248,10 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel, ~ChannelImpl() override { tracing_agent_->disable(); tracing_agent_.reset(); // Dispose before the dispatchers - worker_agent_->disable(); - worker_agent_.reset(); // Dispose before the dispatchers + if (worker_agent_) { + worker_agent_->disable(); + worker_agent_.reset(); // Dispose before the dispatchers + } runtime_agent_->disable(); runtime_agent_.reset(); // Dispose before the dispatchers } @@ -670,6 +674,9 @@ class NodeInspectorClient : public V8InspectorClient { } std::shared_ptr getWorkerManager() { + if (!is_main_) { + return nullptr; + } if (worker_manager_ == nullptr) { worker_manager_ = std::make_shared(getThreadHandle()); @@ -968,7 +975,11 @@ void Agent::SetParentHandle( std::unique_ptr Agent::GetParentHandle( int thread_id, const std::string& url) { - return client_->getWorkerManager()->NewParentHandle(thread_id, url); + if (!parent_handle_) { + return client_->getWorkerManager()->NewParentHandle(thread_id, url); + } else { + return parent_handle_->NewParentInspectorHandle(thread_id, url); + } } void Agent::WaitForConnect() { diff --git a/test/parallel/test-inspector-workers-flat-list.js b/test/parallel/test-inspector-workers-flat-list.js new file mode 100644 index 00000000000000..9a079bf38dead1 --- /dev/null +++ b/test/parallel/test-inspector-workers-flat-list.js @@ -0,0 +1,70 @@ +'use strict'; +const common = require('../common'); +common.skipIfInspectorDisabled(); + +const { Session } = require('inspector'); +const { Worker, isMainThread, parentPort } = require('worker_threads'); + +const MAX_DEPTH = 3; + +let rootWorker = null; + +function runTest() { + let reportedWorkersCount = 0; + const session = new Session(); + session.connect(); + session.on('NodeWorker.attachedToWorker', ({ params: { workerInfo } }) => { + console.log(`Worker ${workerInfo.title} was reported`); + if (++reportedWorkersCount === MAX_DEPTH) { + rootWorker.postMessage({ done: true }); + } + }); + session.post('NodeWorker.enable', { waitForDebuggerOnStart: false }); +} + +function processMessage({ child }) { + console.log(`Worker ${child} is running`); + if (child === MAX_DEPTH) { + runTest(); + } +} + +function workerCallback(message) { + parentPort.postMessage(message); +} + +function startWorker(depth, messageCallback) { + const worker = new Worker(__filename); + worker.on('message', messageCallback); + worker.postMessage({ depth }); + return worker; +} + +function runMainThread() { + rootWorker = startWorker(1, processMessage); +} + +function runWorkerThread() { + let worker = null; + parentPort.on('message', ({ child, depth, done }) => { + if (done) { + if (worker) { + worker.postMessage({ done: true }); + } + parentPort.close(); + } else if (depth) { + parentPort.postMessage({ child: depth }); + if (depth < MAX_DEPTH) { + worker = startWorker(depth + 1, workerCallback); + } + } else if (child) { + parentPort.postMessage({ child }); + } + }); +} + +if (isMainThread) { + runMainThread(); +} else { + runWorkerThread(); +} From 337bc4a53df2822c661c784c0804cb7d7d6e1a3e Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Sun, 28 Jul 2019 16:35:19 -0700 Subject: [PATCH 2/5] squash: address code review comments --- .../test-inspector-workers-flat-list.js | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/test/parallel/test-inspector-workers-flat-list.js b/test/parallel/test-inspector-workers-flat-list.js index 9a079bf38dead1..df6183f35a2ef6 100644 --- a/test/parallel/test-inspector-workers-flat-list.js +++ b/test/parallel/test-inspector-workers-flat-list.js @@ -9,18 +9,19 @@ const MAX_DEPTH = 3; let rootWorker = null; -function runTest() { +const runTest = common.mustCall(function() { let reportedWorkersCount = 0; const session = new Session(); session.connect(); - session.on('NodeWorker.attachedToWorker', ({ params: { workerInfo } }) => { - console.log(`Worker ${workerInfo.title} was reported`); - if (++reportedWorkersCount === MAX_DEPTH) { - rootWorker.postMessage({ done: true }); - } - }); + session.on('NodeWorker.attachedToWorker', common.mustCall( + ({ params: { workerInfo } }) => { + console.log(`Worker ${workerInfo.title} was reported`); + if (++reportedWorkersCount === MAX_DEPTH) { + rootWorker.postMessage({ done: true }); + } + }, MAX_DEPTH)); session.post('NodeWorker.enable', { waitForDebuggerOnStart: false }); -} +}); function processMessage({ child }) { console.log(`Worker ${child} is running`); @@ -44,7 +45,7 @@ function runMainThread() { rootWorker = startWorker(1, processMessage); } -function runWorkerThread() { +function runChildWorkerThread() { let worker = null; parentPort.on('message', ({ child, depth, done }) => { if (done) { @@ -66,5 +67,5 @@ function runWorkerThread() { if (isMainThread) { runMainThread(); } else { - runWorkerThread(); + runChildWorkerThread(); } From 16eee7adb56c2c7bff28fbe9833b402985c49649 Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Mon, 29 Jul 2019 09:15:03 -0700 Subject: [PATCH 3/5] squash: do not use NodeWorker domain in a worker. --- test/parallel/test-inspector-async-hook-after-done.js | 11 +++++++---- test/parallel/test-inspector-workers-flat-list.js | 2 ++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-inspector-async-hook-after-done.js b/test/parallel/test-inspector-async-hook-after-done.js index 10916acd0f8647..9f96fdb7b0da84 100644 --- a/test/parallel/test-inspector-async-hook-after-done.js +++ b/test/parallel/test-inspector-async-hook-after-done.js @@ -3,6 +3,7 @@ const common = require('../common'); common.skipIfInspectorDisabled(); +common.skipIfWorker(); const assert = require('assert'); const { Worker } = require('worker_threads'); @@ -12,9 +13,7 @@ const session = new Session(); let done = false; -session.connect(); - -session.on('NodeWorker.attachedToWorker', ({ params: { sessionId } }) => { +function onAttachToWorker({ params: { sessionId } }) { let id = 1; function postToWorkerInspector(method, params) { session.post('NodeWorker.sendMessageToWorker', { @@ -47,7 +46,11 @@ session.on('NodeWorker.attachedToWorker', ({ params: { sessionId } }) => { { enabled: true }); // start worker postToWorkerInspector('Runtime.runIfWaitingForDebugger'); -}); +} + +session.connect(); + +session.on('NodeWorker.attachedToWorker', common.mustCall(onAttachToWorker)); session.post('NodeWorker.enable', { waitForDebuggerOnStart: true }, () => { new Worker('console.log("Worker is done")', { eval: true }) diff --git a/test/parallel/test-inspector-workers-flat-list.js b/test/parallel/test-inspector-workers-flat-list.js index df6183f35a2ef6..6094e88dd8ae5d 100644 --- a/test/parallel/test-inspector-workers-flat-list.js +++ b/test/parallel/test-inspector-workers-flat-list.js @@ -1,6 +1,8 @@ 'use strict'; const common = require('../common'); + common.skipIfInspectorDisabled(); +common.skipIfWorker(); const { Session } = require('inspector'); const { Worker, isMainThread, parentPort } = require('worker_threads'); From fdd7a2483b4d46f75109d241a11d9e567bd9e3e0 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 29 Jul 2019 14:07:02 -0700 Subject: [PATCH 4/5] fixup --- test/parallel/test-inspector-workers-flat-list.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-inspector-workers-flat-list.js b/test/parallel/test-inspector-workers-flat-list.js index 6094e88dd8ae5d..65aa76bc99260e 100644 --- a/test/parallel/test-inspector-workers-flat-list.js +++ b/test/parallel/test-inspector-workers-flat-list.js @@ -2,10 +2,15 @@ const common = require('../common'); common.skipIfInspectorDisabled(); -common.skipIfWorker(); + +const { Worker, isMainThread, parentPort, workerData } = + require('worker_threads'); + +if (isMainThread || workerData !== 'launched by test') { + common.skipIfWorker(); +} const { Session } = require('inspector'); -const { Worker, isMainThread, parentPort } = require('worker_threads'); const MAX_DEPTH = 3; @@ -37,7 +42,7 @@ function workerCallback(message) { } function startWorker(depth, messageCallback) { - const worker = new Worker(__filename); + const worker = new Worker(__filename, {'launched by test'}); worker.on('message', messageCallback); worker.postMessage({ depth }); return worker; From d80434860ee421a8e950521ac1137b464cc955f2 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 29 Jul 2019 15:40:05 -0700 Subject: [PATCH 5/5] fixup --- test/parallel/test-inspector-workers-flat-list.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-inspector-workers-flat-list.js b/test/parallel/test-inspector-workers-flat-list.js index 65aa76bc99260e..9f6495d10fb147 100644 --- a/test/parallel/test-inspector-workers-flat-list.js +++ b/test/parallel/test-inspector-workers-flat-list.js @@ -42,7 +42,7 @@ function workerCallback(message) { } function startWorker(depth, messageCallback) { - const worker = new Worker(__filename, {'launched by test'}); + const worker = new Worker(__filename, { workerData: 'launched by test' }); worker.on('message', messageCallback); worker.postMessage({ depth }); return worker;