inspector: supported NodeRuntime domain in worker

alexkozy · alexkozy · commit 7e18c650de41 · 2019-06-02T22:37:11.000+03:00
NodeRuntime domain was introduced to give inspector client way to fetch captured information before Node process is gone. We need similar capability for work. With current protocol inspector client can force worker to wait on start by passing waitForDebuggerOnStart flag to NodeWorker.enable method. So client has some time to setup environment, e.g. start profiler. At the same time there is no way to prevent worker from being terminated. So we can start capturing profile but we can not reliably get captured data back. This PR implemented NodeRuntime.notifyWhenWaitingForDisconnect method for worker. When NodeRuntime.waitingForDisconnect notification is enabled, worker will wait for explicit NodeWorker.detach call. With this PR worker tooling story is nicely aligned with main thread tooling story. The only difference is that main thread by default is waiting for disconnect but worker thread is not waiting. Issue: #27677 PR-URL: #27706 Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
diff --git a/src/inspector/node_protocol.pdl b/src/inspector/node_protocol.pdl
@@ -71,6 +71,11 @@ experimental domain NodeWorker
   # Detaches from all running workers and disables attaching to new workers as they are started.
   command disable
 
+  # Detached from the worker with given sessionId.
+  command detach
+    parameters
+      SessionID sessionId
+
   # Issued when attached to a worker.
   event attachedToWorker
     parameters
diff --git a/src/inspector/runtime_agent.cc b/src/inspector/runtime_agent.cc
@@ -16,10 +16,6 @@ void RuntimeAgent::Wire(UberDispatcher* dispatcher) {
 }
 
 DispatchResponse RuntimeAgent::notifyWhenWaitingForDisconnect(bool enabled) {
-  if (!env_->owns_process_state()) {
-    return DispatchResponse::Error(
-        "NodeRuntime domain can only be used through main thread sessions");
-  }
   notify_when_waiting_for_disconnect_ = enabled;
   return DispatchResponse::OK();
 }
diff --git a/src/inspector/worker_agent.cc b/src/inspector/worker_agent.cc
@@ -115,6 +115,11 @@ DispatchResponse WorkerAgent::disable() {
   return DispatchResponse::OK();
 }
 
+DispatchResponse WorkerAgent::detach(const String& sessionId) {
+  workers_->Detached(sessionId);
+  return DispatchResponse::OK();
+}
+
 void NodeWorkers::WorkerCreated(const std::string& title,
                                 const std::string& url,
                                 bool waiting,
diff --git a/src/inspector/worker_agent.h b/src/inspector/worker_agent.h
@@ -25,6 +25,7 @@ class WorkerAgent : public NodeWorker::Backend {
 
   DispatchResponse enable(bool waitForDebuggerOnStart) override;
   DispatchResponse disable() override;
+  DispatchResponse detach(const String& sessionId) override;
 
  private:
   std::shared_ptr<NodeWorker::Frontend> frontend_;
diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc
@@ -472,8 +472,8 @@ class NodeInspectorClient : public V8InspectorClient {
     runMessageLoop();
   }
 
-  void waitForIoShutdown() {
-    waiting_for_io_shutdown_ = true;
+  void waitForSessionsDisconnect() {
+    waiting_for_sessions_disconnect_ = true;
     runMessageLoop();
   }
 
@@ -548,6 +548,8 @@ class NodeInspectorClient : public V8InspectorClient {
       }
       contextDestroyed(env_->context());
     }
+    if (waiting_for_sessions_disconnect_ && !is_main_)
+      waiting_for_sessions_disconnect_ = false;
   }
 
   void dispatchMessageFromFrontend(int session_id, const StringView& message) {
@@ -678,8 +680,9 @@ class NodeInspectorClient : public V8InspectorClient {
   bool shouldRunMessageLoop() {
     if (waiting_for_frontend_)
       return true;
-    if (waiting_for_io_shutdown_ || waiting_for_resume_)
+    if (waiting_for_sessions_disconnect_ || waiting_for_resume_) {
       return hasConnectedSessions();
+    }
     return false;
   }
 
@@ -723,7 +726,7 @@ class NodeInspectorClient : public V8InspectorClient {
   int next_session_id_ = 1;
   bool waiting_for_resume_ = false;
   bool waiting_for_frontend_ = false;
-  bool waiting_for_io_shutdown_ = false;
+  bool waiting_for_sessions_disconnect_ = false;
   // Allows accessing Inspector from non-main threads
   std::unique_ptr<MainThreadInterface> interface_;
   std::shared_ptr<WorkerManager> worker_manager_;
@@ -819,11 +822,14 @@ void Agent::WaitForDisconnect() {
     fprintf(stderr, "Waiting for the debugger to disconnect...\n");
     fflush(stderr);
   }
-  if (!client_->notifyWaitingForDisconnect())
+  if (!client_->notifyWaitingForDisconnect()) {
     client_->contextDestroyed(parent_env_->context());
+  } else if (is_worker) {
+    client_->waitForSessionsDisconnect();
+  }
   if (io_ != nullptr) {
     io_->StopAcceptingNewConnections();
-    client_->waitForIoShutdown();
+    client_->waitForSessionsDisconnect();
   }
 }
 
diff --git a/test/parallel/test-worker-debug.js b/test/parallel/test-worker-debug.js
@@ -206,6 +206,52 @@ async function testTwoWorkers(session, post) {
   await Promise.all([worker1Exited, worker2Exited]);
 }
 
+async function testWaitForDisconnectInWorker(session, post) {
+  console.log('Test NodeRuntime.waitForDisconnect in worker');
+
+  const sessionWithoutWaiting = new Session();
+  sessionWithoutWaiting.connect();
+  const sessionWithoutWaitingPost = doPost.bind(null, sessionWithoutWaiting);
+
+  await sessionWithoutWaitingPost('NodeWorker.enable', {
+    waitForDebuggerOnStart: true
+  });
+  await post('NodeWorker.enable', { waitForDebuggerOnStart: true });
+
+  const attached = [
+    waitForWorkerAttach(session),
+    waitForWorkerAttach(sessionWithoutWaiting)
+  ];
+
+  let worker = null;
+  const exitPromise = runWorker(2, (w) => worker = w);
+
+  const [{ sessionId: sessionId1 }, { sessionId: sessionId2 }] =
+      await Promise.all(attached);
+
+  const workerSession1 = new WorkerSession(session, sessionId1);
+  const workerSession2 = new WorkerSession(sessionWithoutWaiting, sessionId2);
+
+  await workerSession2.post('Runtime.enable');
+  await workerSession1.post('Runtime.enable');
+  await workerSession1.post('NodeRuntime.notifyWhenWaitingForDisconnect', {
+    enabled: true
+  });
+  await workerSession1.post('Runtime.runIfWaitingForDebugger');
+
+  worker.postMessage('resume');
+
+  await waitForEvent(workerSession1, 'NodeRuntime.waitingForDisconnect');
+  post('NodeWorker.detach', { sessionId: sessionId1 });
+  await waitForEvent(workerSession2, 'Runtime.executionContextDestroyed');
+
+  await exitPromise;
+
+  await post('NodeWorker.disable');
+  await sessionWithoutWaitingPost('NodeWorker.disable');
+  sessionWithoutWaiting.disconnect();
+}
+
 async function test() {
   const session = new Session();
   session.connect();
@@ -219,6 +265,7 @@ async function test() {
 
   await testNoWaitOnStart(session, post);
   await testTwoWorkers(session, post);
+  await testWaitForDisconnectInWorker(session, post);
 
   session.disconnect();
   console.log('Test done');

Original file line number	Diff line number	Diff line change
`@@ -16,10 +16,6 @@ void RuntimeAgent::Wire(UberDispatcher* dispatcher) {`
`16`	`16`	`}`
`17`	`17`
`18`	`18`	`DispatchResponse RuntimeAgent::notifyWhenWaitingForDisconnect(bool enabled) {`
`19`		`- if (!env_->owns_process_state()) {`
`20`		`- return DispatchResponse::Error(`
`21`		`- "NodeRuntime domain can only be used through main thread sessions");`
`22`		`- }`
`23`	`19`	`notify_when_waiting_for_disconnect_ = enabled;`
`24`	`20`	`return DispatchResponse::OK();`
`25`	`21`	`}`
Original file line number	Diff line number	Diff line change
`@@ -472,8 +472,8 @@ class NodeInspectorClient : public V8InspectorClient {`
`472`	`472`	`runMessageLoop();`
`473`	`473`	`}`
`474`	`474`
`475`		`- void waitForIoShutdown() {`
`476`		`- waiting_for_io_shutdown_ = true;`
	`475`	`+ void waitForSessionsDisconnect() {`
	`476`	`+ waiting_for_sessions_disconnect_ = true;`
`477`	`477`	`runMessageLoop();`
`478`	`478`	`}`
`479`	`479`
`@@ -548,6 +548,8 @@ class NodeInspectorClient : public V8InspectorClient {`
`548`	`548`	`}`
`549`	`549`	`contextDestroyed(env_->context());`
`550`	`550`	`}`
	`551`	`+ if (waiting_for_sessions_disconnect_ && !is_main_)`
	`552`	`+ waiting_for_sessions_disconnect_ = false;`
`551`	`553`	`}`
`552`	`554`
`553`	`555`	`void dispatchMessageFromFrontend(int session_id, const StringView& message) {`
`@@ -678,8 +680,9 @@ class NodeInspectorClient : public V8InspectorClient {`
`678`	`680`	`bool shouldRunMessageLoop() {`
`679`	`681`	`if (waiting_for_frontend_)`
`680`	`682`	`return true;`
`681`		`- if (waiting_for_io_shutdown_ \|\| waiting_for_resume_)`
	`683`	`+ if (waiting_for_sessions_disconnect_ \|\| waiting_for_resume_) {`
`682`	`684`	`return hasConnectedSessions();`
	`685`	`+ }`
`683`	`686`	`return false;`
`684`	`687`	`}`
`685`	`688`
`@@ -723,7 +726,7 @@ class NodeInspectorClient : public V8InspectorClient {`
`723`	`726`	`int next_session_id_ = 1;`
`724`	`727`	`bool waiting_for_resume_ = false;`
`725`	`728`	`bool waiting_for_frontend_ = false;`
`726`		`- bool waiting_for_io_shutdown_ = false;`
	`729`	`+ bool waiting_for_sessions_disconnect_ = false;`
`727`	`730`	`// Allows accessing Inspector from non-main threads`
`728`	`731`	`std::unique_ptr<MainThreadInterface> interface_;`
`729`	`732`	`std::shared_ptr<WorkerManager> worker_manager_;`
`@@ -819,11 +822,14 @@ void Agent::WaitForDisconnect() {`
`819`	`822`	`fprintf(stderr, "Waiting for the debugger to disconnect...\n");`
`820`	`823`	`fflush(stderr);`
`821`	`824`	`}`
`822`		`- if (!client_->notifyWaitingForDisconnect())`
	`825`	`+ if (!client_->notifyWaitingForDisconnect()) {`
`823`	`826`	`client_->contextDestroyed(parent_env_->context());`
	`827`	`+ } else if (is_worker) {`
	`828`	`+ client_->waitForSessionsDisconnect();`
	`829`	`+ }`
`824`	`830`	`if (io_ != nullptr) {`
`825`	`831`	`io_->StopAcceptingNewConnections();`
`826`		`- client_->waitForIoShutdown();`
	`832`	`+ client_->waitForSessionsDisconnect();`
`827`	`833`	`}`
`828`	`834`	`}`
`829`	`835`