Skip to content

Commit 376ac5f

Browse files
Eugene Ostroukhovjasnell
Eugene Ostroukhov
authored andcommitted
inspector: Allows reentry when paused
This change allows reentering the message dispatch loop when the Node is paused. This is necessary when the pause happened as a result of the message sent by a debug frontend, such as evaluating a function with a breakpoint inside. Fixes: #13320 PR-URL: #13350 Reviewed-By: James M Snell <[email protected]>
1 parent 414da1b commit 376ac5f

7 files changed

+181
-24
lines changed

src/inspector_agent.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ class JsBindingsSessionDelegate : public InspectorSessionDelegate {
203203
callback_.Reset();
204204
}
205205

206-
bool WaitForFrontendMessage() override {
206+
bool WaitForFrontendMessageWhilePaused() override {
207207
return false;
208208
}
209209

@@ -393,7 +393,7 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel {
393393
}
394394

395395
bool waitForFrontendMessage() {
396-
return delegate_->WaitForFrontendMessage();
396+
return delegate_->WaitForFrontendMessageWhilePaused();
397397
}
398398

399399
void schedulePauseOnNextStatement(const std::string& reason) {

src/inspector_agent.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ namespace inspector {
3838
class InspectorSessionDelegate {
3939
public:
4040
virtual ~InspectorSessionDelegate() = default;
41-
virtual bool WaitForFrontendMessage() = 0;
41+
virtual bool WaitForFrontendMessageWhilePaused() = 0;
4242
virtual void SendMessageToFrontend(const v8_inspector::StringView& message)
4343
= 0;
4444
};

src/inspector_io.cc

+14-9
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ std::unique_ptr<StringBuffer> Utf8ToStringView(const std::string& message) {
134134
class IoSessionDelegate : public InspectorSessionDelegate {
135135
public:
136136
explicit IoSessionDelegate(InspectorIo* io) : io_(io) { }
137-
bool WaitForFrontendMessage() override;
137+
bool WaitForFrontendMessageWhilePaused() override;
138138
void SendMessageToFrontend(const v8_inspector::StringView& message) override;
139139
private:
140140
InspectorIo* io_;
@@ -354,7 +354,8 @@ void InspectorIo::PostIncomingMessage(InspectorAction action, int session_id,
354354
NotifyMessageReceived();
355355
}
356356

357-
void InspectorIo::WaitForIncomingMessage() {
357+
void InspectorIo::WaitForFrontendMessageWhilePaused() {
358+
dispatching_messages_ = false;
358359
Mutex::ScopedLock scoped_lock(state_lock_);
359360
if (incoming_message_queue_.empty())
360361
incoming_message_cond_.Wait(scoped_lock);
@@ -373,11 +374,15 @@ void InspectorIo::DispatchMessages() {
373374
if (dispatching_messages_)
374375
return;
375376
dispatching_messages_ = true;
376-
MessageQueue<InspectorAction> tasks;
377+
bool had_messages = false;
377378
do {
378-
tasks.clear();
379-
SwapBehindLock(&incoming_message_queue_, &tasks);
380-
for (const auto& task : tasks) {
379+
if (dispatching_message_queue_.empty())
380+
SwapBehindLock(&incoming_message_queue_, &dispatching_message_queue_);
381+
had_messages = !dispatching_message_queue_.empty();
382+
while (!dispatching_message_queue_.empty()) {
383+
MessageQueue<InspectorAction>::value_type task;
384+
std::swap(dispatching_message_queue_.front(), task);
385+
dispatching_message_queue_.pop_front();
381386
StringView message = std::get<2>(task)->string();
382387
switch (std::get<0>(task)) {
383388
case InspectorAction::kStartSession:
@@ -404,7 +409,7 @@ void InspectorIo::DispatchMessages() {
404409
break;
405410
}
406411
}
407-
} while (!tasks.empty());
412+
} while (had_messages);
408413
dispatching_messages_ = false;
409414
}
410415

@@ -485,8 +490,8 @@ std::string InspectorIoDelegate::GetTargetUrl(const std::string& id) {
485490
return "file://" + script_path_;
486491
}
487492

488-
bool IoSessionDelegate::WaitForFrontendMessage() {
489-
io_->WaitForIncomingMessage();
493+
bool IoSessionDelegate::WaitForFrontendMessageWhilePaused() {
494+
io_->WaitForFrontendMessageWhilePaused();
490495
return true;
491496
}
492497

src/inspector_io.h

+4-3
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
#include "node_mutex.h"
77
#include "uv.h"
88

9+
#include <deque>
910
#include <memory>
1011
#include <stddef.h>
11-
#include <vector>
1212

1313
#if !HAVE_INSPECTOR
1414
#error("This header can only be used when inspector is enabled")
@@ -76,7 +76,7 @@ class InspectorIo {
7676
private:
7777
template <typename Action>
7878
using MessageQueue =
79-
std::vector<std::tuple<Action, int,
79+
std::deque<std::tuple<Action, int,
8080
std::unique_ptr<v8_inspector::StringBuffer>>>;
8181
enum class State {
8282
kNew,
@@ -115,7 +115,7 @@ class InspectorIo {
115115
void SwapBehindLock(MessageQueue<ActionType>* vector1,
116116
MessageQueue<ActionType>* vector2);
117117
// Wait on incoming_message_cond_
118-
void WaitForIncomingMessage();
118+
void WaitForFrontendMessageWhilePaused();
119119
// Broadcast incoming_message_cond_
120120
void NotifyMessageReceived();
121121

@@ -145,6 +145,7 @@ class InspectorIo {
145145
Mutex state_lock_; // Locked before mutating either queue.
146146
MessageQueue<InspectorAction> incoming_message_queue_;
147147
MessageQueue<TransportAction> outgoing_message_queue_;
148+
MessageQueue<InspectorAction> dispatching_message_queue_;
148149

149150
bool dispatching_messages_;
150151
int session_id_;

test/inspector/global-function.js

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
'use strict'; // eslint-disable-line required-modules
2+
let invocations = 0;
3+
const interval = setInterval(() => {}, 1000);
4+
5+
global.sum = function() {
6+
const a = 1;
7+
const b = 2;
8+
const c = a + b;
9+
clearInterval(interval);
10+
console.log(invocations++, c);
11+
};
12+
13+
console.log('Ready!');

test/inspector/inspector-helper.js

+19-9
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const url = require('url');
1010
const DEBUG = false;
1111
const TIMEOUT = 15 * 1000;
1212
const EXPECT_ALIVE_SYMBOL = Symbol('isAlive');
13+
const DONT_EXPECT_RESPONSE_SYMBOL = Symbol('dontExpectResponse');
1314
const mainScript = path.join(common.fixturesDir, 'loop.js');
1415

1516
function send(socket, message, id, callback) {
@@ -183,7 +184,6 @@ TestSession.prototype.processMessage_ = function(message) {
183184
this.messagefilter_ && this.messagefilter_(message);
184185
const id = message['id'];
185186
if (id) {
186-
assert.strictEqual(id, this.expectedId_);
187187
this.expectedId_++;
188188
if (this.responseCheckers_[id]) {
189189
const messageJSON = JSON.stringify(message);
@@ -207,16 +207,21 @@ TestSession.prototype.sendAll_ = function(commands, callback) {
207207
if (!commands.length) {
208208
callback();
209209
} else {
210-
this.lastId_++;
210+
let id = ++this.lastId_;
211211
let command = commands[0];
212212
if (command instanceof Array) {
213-
this.responseCheckers_[this.lastId_] = command[1];
213+
this.responseCheckers_[id] = command[1];
214214
command = command[0];
215215
}
216216
if (command instanceof Function)
217217
command = command();
218-
this.messages_[this.lastId_] = command;
219-
send(this.socket_, command, this.lastId_,
218+
if (!command[DONT_EXPECT_RESPONSE_SYMBOL]) {
219+
this.messages_[id] = command;
220+
} else {
221+
id += 100000;
222+
this.lastId_--;
223+
}
224+
send(this.socket_, command, id,
220225
() => this.sendAll_(commands.slice(1), callback));
221226
}
222227
};
@@ -497,12 +502,13 @@ Harness.prototype.kill = function() {
497502

498503
exports.startNodeForInspectorTest = function(callback,
499504
inspectorFlags = ['--inspect-brk'],
500-
opt_script_contents) {
505+
scriptContents = '',
506+
scriptFile = mainScript) {
501507
const args = [].concat(inspectorFlags);
502-
if (opt_script_contents) {
503-
args.push('-e', opt_script_contents);
508+
if (scriptContents) {
509+
args.push('-e', scriptContents);
504510
} else {
505-
args.push(mainScript);
511+
args.push(scriptFile);
506512
}
507513

508514
const child = spawn(process.execPath, args);
@@ -534,3 +540,7 @@ exports.startNodeForInspectorTest = function(callback,
534540
exports.mainScriptSource = function() {
535541
return fs.readFileSync(mainScript, 'utf8');
536542
};
543+
544+
exports.markMessageNoResponse = function(message) {
545+
message[DONT_EXPECT_RESPONSE_SYMBOL] = true;
546+
};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
'use strict';
2+
const common = require('../common');
3+
common.skipIfInspectorDisabled();
4+
const assert = require('assert');
5+
const helper = require('./inspector-helper.js');
6+
const path = require('path');
7+
8+
const script = path.join(path.dirname(module.filename), 'global-function.js');
9+
10+
11+
function setupExpectBreakOnLine(line, url, session) {
12+
return function(message) {
13+
if ('Debugger.paused' === message['method']) {
14+
const callFrame = message['params']['callFrames'][0];
15+
const location = callFrame['location'];
16+
assert.strictEqual(url, session.scriptUrlForId(location['scriptId']));
17+
assert.strictEqual(line, location['lineNumber']);
18+
return true;
19+
}
20+
};
21+
}
22+
23+
function setupExpectConsoleOutputAndBreak(type, values) {
24+
if (!(values instanceof Array))
25+
values = [ values ];
26+
let consoleLog = false;
27+
function matchConsoleLog(message) {
28+
if ('Runtime.consoleAPICalled' === message['method']) {
29+
const params = message['params'];
30+
if (params['type'] === type) {
31+
let i = 0;
32+
for (const value of params['args']) {
33+
if (value['value'] !== values[i++])
34+
return false;
35+
}
36+
return i === values.length;
37+
}
38+
}
39+
}
40+
41+
return function(message) {
42+
if (consoleLog)
43+
return message['method'] === 'Debugger.paused';
44+
consoleLog = matchConsoleLog(message);
45+
return false;
46+
};
47+
}
48+
49+
function setupExpectContextDestroyed(id) {
50+
return function(message) {
51+
if ('Runtime.executionContextDestroyed' === message['method'])
52+
return message['params']['executionContextId'] === id;
53+
};
54+
}
55+
56+
function setupDebugger(session) {
57+
console.log('[test]', 'Setting up a debugger');
58+
const commands = [
59+
{ 'method': 'Runtime.enable' },
60+
{ 'method': 'Debugger.enable' },
61+
{ 'method': 'Debugger.setAsyncCallStackDepth',
62+
'params': {'maxDepth': 0} },
63+
{ 'method': 'Runtime.runIfWaitingForDebugger' },
64+
];
65+
66+
session
67+
.sendInspectorCommands(commands)
68+
.expectMessages((message) => 'Runtime.consoleAPICalled' === message.method);
69+
}
70+
71+
function breakOnLine(session) {
72+
console.log('[test]', 'Breaking in the code');
73+
const commands = [
74+
{ 'method': 'Debugger.setBreakpointByUrl',
75+
'params': { 'lineNumber': 9,
76+
'url': script,
77+
'columnNumber': 0,
78+
'condition': ''
79+
}
80+
},
81+
{ 'method': 'Runtime.evaluate',
82+
'params': { 'expression': 'sum()',
83+
'objectGroup': 'console',
84+
'includeCommandLineAPI': true,
85+
'silent': false,
86+
'contextId': 1,
87+
'returnByValue': false,
88+
'generatePreview': true,
89+
'userGesture': true,
90+
'awaitPromise': false
91+
}
92+
}
93+
];
94+
helper.markMessageNoResponse(commands[1]);
95+
session
96+
.sendInspectorCommands(commands)
97+
.expectMessages(setupExpectBreakOnLine(9, script, session));
98+
}
99+
100+
function stepOverConsoleStatement(session) {
101+
console.log('[test]', 'Step over console statement and test output');
102+
session
103+
.sendInspectorCommands({ 'method': 'Debugger.stepOver' })
104+
.expectMessages(setupExpectConsoleOutputAndBreak('log', [0, 3]));
105+
}
106+
107+
function testWaitsForFrontendDisconnect(session, harness) {
108+
console.log('[test]', 'Verify node waits for the frontend to disconnect');
109+
session.sendInspectorCommands({ 'method': 'Debugger.resume'})
110+
.expectMessages(setupExpectContextDestroyed(1))
111+
.expectStderrOutput('Waiting for the debugger to disconnect...')
112+
.disconnect(true);
113+
}
114+
115+
function runTests(harness) {
116+
harness
117+
.runFrontendSession([
118+
setupDebugger,
119+
breakOnLine,
120+
stepOverConsoleStatement,
121+
testWaitsForFrontendDisconnect
122+
]).expectShutDown(0);
123+
}
124+
125+
helper.startNodeForInspectorTest(runTests,
126+
['--inspect'],
127+
undefined,
128+
script);

0 commit comments

Comments
 (0)