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

inspector: add NodeRuntime.waitingForDebugger event · nodejs/node@3fe59ba · GitHub

/ node Public

Commit 3fe59ba

Browse files
mmarchini
authored and
richardlau
committed
inspector: add NodeRuntime.waitingForDebugger event
`NodeRuntime.waitingForDebugger` is a new Inspector Protocol event that will fire when the process being inspected is waiting for the debugger (for example, when `inspector.waitForDebugger()` is called). This allows inspecting processes to know when the inspected process is waiting for a `Runtime.runIfWaitingForDebugger` message to resume execution. It allows tooling to resume execution of the inspected process as soon as it deems necessary, without having to guess if the inspected process is waiting or not, making the workflow more deterministic. With a more deterministic workflow, it is possible to update Node.js core tests to avoid race conditions that can cause flakiness. Therefore, tests were also changed as following: * Remove no-op Runtime.runIfWaitingForDebugger from tests that don't need it * Use NodeRuntime.waitingForDebugger in all tests that need Runtime.runIfWaitingForDebugger, to ensure order of operations is predictable and correct * Simplify test-inspector-multisession-ws There might be value in adding `NodeWorker.waitingForDebugger` in a future patch, but as of right now, no Node.js core inspector tests using worker threads are not failing due to race conditions. Fixes: #34730 PR-URL: #51560 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent 06a29f8 commit 3fe59ba

25 files changed

Lines changed: 160 additions & 28 deletions

‎src/inspector/node_protocol.pdl‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,12 @@ experimental domain NodeWorker
100100

101101
# Support for inspecting node process state.
102102
experimental domain NodeRuntime
103+
# Enable the NodeRuntime events except by `NodeRuntime.waitingForDisconnect`.
104+
command enable
105+
106+
# Disable NodeRuntime events
107+
command disable
108+
103109
# Enable the `NodeRuntime.waitingForDisconnect`.
104110
command notifyWhenWaitingForDisconnect
105111
parameters
@@ -110,3 +116,7 @@ experimental domain NodeRuntime
110116
# It is fired when the Node process finished all code execution and is
111117
# waiting for all frontends to disconnect.
112118
event waitingForDisconnect
119+
120+
# This event is fired when the runtime is waiting for the debugger. For
121+
# example, when inspector.waitingForDebugger is called
122+
event waitingForDebugger

‎src/inspector/runtime_agent.cc‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ namespace inspector {
88
namespace protocol {
99

1010
RuntimeAgent::RuntimeAgent()
11-
: notify_when_waiting_for_disconnect_(false) {}
11+
: notify_when_waiting_for_disconnect_(false),
12+
enabled_(false),
13+
is_waiting_for_debugger_(false) {}
1214

1315
void RuntimeAgent::Wire(UberDispatcher* dispatcher) {
1416
frontend_ = std::make_unique<NodeRuntime::Frontend>(dispatcher->channel());
@@ -20,6 +22,30 @@ DispatchResponse RuntimeAgent::notifyWhenWaitingForDisconnect(bool enabled) {
2022
return DispatchResponse::OK();
2123
}
2224

25+
DispatchResponse RuntimeAgent::enable() {
26+
enabled_ = true;
27+
if (is_waiting_for_debugger_) {
28+
frontend_->waitingForDebugger();
29+
}
30+
return DispatchResponse::OK();
31+
}
32+
33+
DispatchResponse RuntimeAgent::disable() {
34+
enabled_ = false;
35+
return DispatchResponse::OK();
36+
}
37+
38+
void RuntimeAgent::setWaitingForDebugger() {
39+
is_waiting_for_debugger_ = true;
40+
if (enabled_) {
41+
frontend_->waitingForDebugger();
42+
}
43+
}
44+
45+
void RuntimeAgent::unsetWaitingForDebugger() {
46+
is_waiting_for_debugger_ = false;
47+
}
48+
2349
bool RuntimeAgent::notifyWaitingForDisconnect() {
2450
if (notify_when_waiting_for_disconnect_) {
2551
frontend_->waitingForDisconnect();

‎src/inspector/runtime_agent.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,21 @@ class RuntimeAgent : public NodeRuntime::Backend {
1818

1919
DispatchResponse notifyWhenWaitingForDisconnect(bool enabled) override;
2020

21+
DispatchResponse enable() override;
22+
23+
DispatchResponse disable() override;
24+
2125
bool notifyWaitingForDisconnect();
2226

27+
void setWaitingForDebugger();
28+
29+
void unsetWaitingForDebugger();
30+
2331
private:
2432
std::shared_ptr<NodeRuntime::Frontend> frontend_;
2533
bool notify_when_waiting_for_disconnect_;
34+
bool enabled_;
35+
bool is_waiting_for_debugger_;
2636
};
2737
} // namespace protocol
2838
} // namespace inspector

‎src/inspector_agent.cc‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,10 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel,
278278
return retaining_context_;
279279
}
280280

281+
void setWaitingForDebugger() { runtime_agent_->setWaitingForDebugger(); }
282+
283+
void unsetWaitingForDebugger() { runtime_agent_->unsetWaitingForDebugger(); }
284+
281285
bool retainingContext() {
282286
return retaining_context_;
283287
}
@@ -418,6 +422,9 @@ class NodeInspectorClient : public V8InspectorClient {
418422

419423
void waitForFrontend() {
420424
waiting_for_frontend_ = true;
425+
for (const auto& id_channel : channels_) {
426+
id_channel.second->setWaitingForDebugger();
427+
}
421428
runMessageLoop();
422429
}
423430

@@ -465,6 +472,9 @@ class NodeInspectorClient : public V8InspectorClient {
465472

466473
void runIfWaitingForDebugger(int context_group_id) override {
467474
waiting_for_frontend_ = false;
475+
for (const auto& id_channel : channels_) {
476+
id_channel.second->unsetWaitingForDebugger();
477+
}
468478
}
469479

470480
int connectFrontend(std::unique_ptr<InspectorSessionDelegate> delegate,
@@ -476,6 +486,9 @@ class NodeInspectorClient : public V8InspectorClient {
476486
std::move(delegate),
477487
getThreadHandle(),
478488
prevent_shutdown);
489+
if (waiting_for_frontend_) {
490+
channels_[session_id]->setWaitingForDebugger();
491+
}
479492
return session_id;
480493
}
481494

‎test/common/inspector-helper.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,9 @@ class InspectorSession {
303303
console.log('[test]', 'Verify node waits for the frontend to disconnect');
304304
await this.send({ 'method': 'Debugger.resume' });
305305
await this.waitForNotification((notification) => {
306+
if (notification.method === 'Debugger.paused') {
307+
this.send({ 'method': 'Debugger.resume' });
308+
}
306309
return notification.method === 'Runtime.executionContextDestroyed' &&
307310
notification.params.executionContextId === 1;
308311
});

‎test/parallel/test-inspect-async-hook-setup-at-inspect.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ async function runTests() {
5454
'params': { 'maxDepth': 10 } },
5555
{ 'method': 'Debugger.setBlackboxPatterns',
5656
'params': { 'patterns': [] } },
57-
{ 'method': 'Runtime.runIfWaitingForDebugger' },
5857
]);
5958

6059
await waitForInitialSetup(session);

‎test/parallel/test-inspector-async-hook-setup-at-inspect-brk.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ async function checkAsyncStackTrace(session) {
3030
async function runTests() {
3131
const instance = new NodeInstance(undefined, script);
3232
const session = await instance.connectInspectorSession();
33+
await session.send({ method: 'NodeRuntime.enable' });
34+
await session.waitForNotification('NodeRuntime.waitingForDebugger');
3335
await session.send([
3436
{ 'method': 'Runtime.enable' },
3537
{ 'method': 'Debugger.enable' },
@@ -39,6 +41,7 @@ async function runTests() {
3941
'params': { 'patterns': [] } },
4042
{ 'method': 'Runtime.runIfWaitingForDebugger' },
4143
]);
44+
await session.send({ method: 'NodeRuntime.disable' });
4245
await skipBreakpointAtStart(session);
4346
await checkAsyncStackTrace(session);
4447

‎test/parallel/test-inspector-async-hook-setup-at-signal.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ async function runTests() {
6969
'params': { 'maxDepth': 10 } },
7070
{ 'method': 'Debugger.setBlackboxPatterns',
7171
'params': { 'patterns': [] } },
72-
{ 'method': 'Runtime.runIfWaitingForDebugger' },
7372
]);
7473

7574
await waitForInitialSetup(session);

‎test/parallel/test-inspector-async-stack-traces-promise-then.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ function runTest() {
2020
async function runTests() {
2121
const instance = new NodeInstance(undefined, script);
2222
const session = await instance.connectInspectorSession();
23+
await session.send({ method: 'NodeRuntime.enable' });
24+
await session.waitForNotification('NodeRuntime.waitingForDebugger');
2325
await session.send([
2426
{ 'method': 'Runtime.enable' },
2527
{ 'method': 'Debugger.enable' },
@@ -29,6 +31,7 @@ async function runTests() {
2931
'params': { 'patterns': [] } },
3032
{ 'method': 'Runtime.runIfWaitingForDebugger' },
3133
]);
34+
session.send({ method: 'NodeRuntime.disable' });
3235

3336
await session.waitForBreakOnLine(0, '[eval]');
3437
await session.send({ 'method': 'Debugger.resume' });

‎test/parallel/test-inspector-async-stack-traces-set-interval.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ async function checkAsyncStackTrace(session) {
2626
async function runTests() {
2727
const instance = new NodeInstance(undefined, script);
2828
const session = await instance.connectInspectorSession();
29+
await session.send({ method: 'NodeRuntime.enable' });
30+
await session.waitForNotification('NodeRuntime.waitingForDebugger');
2931
await session.send([
3032
{ 'method': 'Runtime.enable' },
3133
{ 'method': 'Debugger.enable' },
@@ -35,6 +37,7 @@ async function runTests() {
3537
'params': { 'patterns': [] } },
3638
{ 'method': 'Runtime.runIfWaitingForDebugger' },
3739
]);
40+
await session.send({ method: 'NodeRuntime.disable' });
3841

3942
await skipFirstBreakpoint(session);
4043
await checkAsyncStackTrace(session);

0 commit comments

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