Skip to content

Commit

Permalink
inspector: Allows reentry when paused
Browse files Browse the repository at this point in the history
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 <jasnell@gmail.com>
  • Loading branch information
Eugene Ostroukhov authored and jasnell committed Jun 5, 2017
1 parent 414da1b commit 376ac5f
Show file tree
Hide file tree
Showing 7 changed files with 181 additions and 24 deletions.
4 changes: 2 additions & 2 deletions src/inspector_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ class JsBindingsSessionDelegate : public InspectorSessionDelegate {
callback_.Reset();
}

bool WaitForFrontendMessage() override {
bool WaitForFrontendMessageWhilePaused() override {
return false;
}

Expand Down Expand Up @@ -393,7 +393,7 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel {
}

bool waitForFrontendMessage() {
return delegate_->WaitForFrontendMessage();
return delegate_->WaitForFrontendMessageWhilePaused();
}

void schedulePauseOnNextStatement(const std::string& reason) {
Expand Down
2 changes: 1 addition & 1 deletion src/inspector_agent.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ namespace inspector {
class InspectorSessionDelegate {
public:
virtual ~InspectorSessionDelegate() = default;
virtual bool WaitForFrontendMessage() = 0;
virtual bool WaitForFrontendMessageWhilePaused() = 0;
virtual void SendMessageToFrontend(const v8_inspector::StringView& message)
= 0;
};
Expand Down
23 changes: 14 additions & 9 deletions src/inspector_io.cc
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ std::unique_ptr<StringBuffer> Utf8ToStringView(const std::string& message) {
class IoSessionDelegate : public InspectorSessionDelegate {
public:
explicit IoSessionDelegate(InspectorIo* io) : io_(io) { }
bool WaitForFrontendMessage() override;
bool WaitForFrontendMessageWhilePaused() override;
void SendMessageToFrontend(const v8_inspector::StringView& message) override;
private:
InspectorIo* io_;
Expand Down Expand Up @@ -354,7 +354,8 @@ void InspectorIo::PostIncomingMessage(InspectorAction action, int session_id,
NotifyMessageReceived();
}

void InspectorIo::WaitForIncomingMessage() {
void InspectorIo::WaitForFrontendMessageWhilePaused() {
dispatching_messages_ = false;
Mutex::ScopedLock scoped_lock(state_lock_);
if (incoming_message_queue_.empty())
incoming_message_cond_.Wait(scoped_lock);
Expand All @@ -373,11 +374,15 @@ void InspectorIo::DispatchMessages() {
if (dispatching_messages_)
return;
dispatching_messages_ = true;
MessageQueue<InspectorAction> tasks;
bool had_messages = false;
do {
tasks.clear();
SwapBehindLock(&incoming_message_queue_, &tasks);
for (const auto& task : tasks) {
if (dispatching_message_queue_.empty())
SwapBehindLock(&incoming_message_queue_, &dispatching_message_queue_);
had_messages = !dispatching_message_queue_.empty();
while (!dispatching_message_queue_.empty()) {
MessageQueue<InspectorAction>::value_type task;
std::swap(dispatching_message_queue_.front(), task);
dispatching_message_queue_.pop_front();
StringView message = std::get<2>(task)->string();
switch (std::get<0>(task)) {
case InspectorAction::kStartSession:
Expand All @@ -404,7 +409,7 @@ void InspectorIo::DispatchMessages() {
break;
}
}
} while (!tasks.empty());
} while (had_messages);
dispatching_messages_ = false;
}

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

bool IoSessionDelegate::WaitForFrontendMessage() {
io_->WaitForIncomingMessage();
bool IoSessionDelegate::WaitForFrontendMessageWhilePaused() {
io_->WaitForFrontendMessageWhilePaused();
return true;
}

Expand Down
7 changes: 4 additions & 3 deletions src/inspector_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
#include "node_mutex.h"
#include "uv.h"

#include <deque>
#include <memory>
#include <stddef.h>
#include <vector>

#if !HAVE_INSPECTOR
#error("This header can only be used when inspector is enabled")
Expand Down Expand Up @@ -76,7 +76,7 @@ class InspectorIo {
private:
template <typename Action>
using MessageQueue =
std::vector<std::tuple<Action, int,
std::deque<std::tuple<Action, int,
std::unique_ptr<v8_inspector::StringBuffer>>>;
enum class State {
kNew,
Expand Down Expand Up @@ -115,7 +115,7 @@ class InspectorIo {
void SwapBehindLock(MessageQueue<ActionType>* vector1,
MessageQueue<ActionType>* vector2);
// Wait on incoming_message_cond_
void WaitForIncomingMessage();
void WaitForFrontendMessageWhilePaused();
// Broadcast incoming_message_cond_
void NotifyMessageReceived();

Expand Down Expand Up @@ -145,6 +145,7 @@ class InspectorIo {
Mutex state_lock_; // Locked before mutating either queue.
MessageQueue<InspectorAction> incoming_message_queue_;
MessageQueue<TransportAction> outgoing_message_queue_;
MessageQueue<InspectorAction> dispatching_message_queue_;

bool dispatching_messages_;
int session_id_;
Expand Down
13 changes: 13 additions & 0 deletions test/inspector/global-function.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use strict'; // eslint-disable-line required-modules
let invocations = 0;
const interval = setInterval(() => {}, 1000);

global.sum = function() {
const a = 1;
const b = 2;
const c = a + b;
clearInterval(interval);
console.log(invocations++, c);
};

console.log('Ready!');
28 changes: 19 additions & 9 deletions test/inspector/inspector-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const url = require('url');
const DEBUG = false;
const TIMEOUT = 15 * 1000;
const EXPECT_ALIVE_SYMBOL = Symbol('isAlive');
const DONT_EXPECT_RESPONSE_SYMBOL = Symbol('dontExpectResponse');
const mainScript = path.join(common.fixturesDir, 'loop.js');

function send(socket, message, id, callback) {
Expand Down Expand Up @@ -183,7 +184,6 @@ TestSession.prototype.processMessage_ = function(message) {
this.messagefilter_ && this.messagefilter_(message);
const id = message['id'];
if (id) {
assert.strictEqual(id, this.expectedId_);
this.expectedId_++;
if (this.responseCheckers_[id]) {
const messageJSON = JSON.stringify(message);
Expand All @@ -207,16 +207,21 @@ TestSession.prototype.sendAll_ = function(commands, callback) {
if (!commands.length) {
callback();
} else {
this.lastId_++;
let id = ++this.lastId_;
let command = commands[0];
if (command instanceof Array) {
this.responseCheckers_[this.lastId_] = command[1];
this.responseCheckers_[id] = command[1];
command = command[0];
}
if (command instanceof Function)
command = command();
this.messages_[this.lastId_] = command;
send(this.socket_, command, this.lastId_,
if (!command[DONT_EXPECT_RESPONSE_SYMBOL]) {
this.messages_[id] = command;
} else {
id += 100000;
this.lastId_--;
}
send(this.socket_, command, id,
() => this.sendAll_(commands.slice(1), callback));
}
};
Expand Down Expand Up @@ -497,12 +502,13 @@ Harness.prototype.kill = function() {

exports.startNodeForInspectorTest = function(callback,
inspectorFlags = ['--inspect-brk'],
opt_script_contents) {
scriptContents = '',
scriptFile = mainScript) {
const args = [].concat(inspectorFlags);
if (opt_script_contents) {
args.push('-e', opt_script_contents);
if (scriptContents) {
args.push('-e', scriptContents);
} else {
args.push(mainScript);
args.push(scriptFile);
}

const child = spawn(process.execPath, args);
Expand Down Expand Up @@ -534,3 +540,7 @@ exports.startNodeForInspectorTest = function(callback,
exports.mainScriptSource = function() {
return fs.readFileSync(mainScript, 'utf8');
};

exports.markMessageNoResponse = function(message) {
message[DONT_EXPECT_RESPONSE_SYMBOL] = true;
};
128 changes: 128 additions & 0 deletions test/inspector/test-inspector-break-when-eval.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
'use strict';
const common = require('../common');
common.skipIfInspectorDisabled();
const assert = require('assert');
const helper = require('./inspector-helper.js');
const path = require('path');

const script = path.join(path.dirname(module.filename), 'global-function.js');


function setupExpectBreakOnLine(line, url, session) {
return function(message) {
if ('Debugger.paused' === message['method']) {
const callFrame = message['params']['callFrames'][0];
const location = callFrame['location'];
assert.strictEqual(url, session.scriptUrlForId(location['scriptId']));
assert.strictEqual(line, location['lineNumber']);
return true;
}
};
}

function setupExpectConsoleOutputAndBreak(type, values) {
if (!(values instanceof Array))
values = [ values ];
let consoleLog = false;
function matchConsoleLog(message) {
if ('Runtime.consoleAPICalled' === message['method']) {
const params = message['params'];
if (params['type'] === type) {
let i = 0;
for (const value of params['args']) {
if (value['value'] !== values[i++])
return false;
}
return i === values.length;
}
}
}

return function(message) {
if (consoleLog)
return message['method'] === 'Debugger.paused';
consoleLog = matchConsoleLog(message);
return false;
};
}

function setupExpectContextDestroyed(id) {
return function(message) {
if ('Runtime.executionContextDestroyed' === message['method'])
return message['params']['executionContextId'] === id;
};
}

function setupDebugger(session) {
console.log('[test]', 'Setting up a debugger');
const commands = [
{ 'method': 'Runtime.enable' },
{ 'method': 'Debugger.enable' },
{ 'method': 'Debugger.setAsyncCallStackDepth',
'params': {'maxDepth': 0} },
{ 'method': 'Runtime.runIfWaitingForDebugger' },
];

session
.sendInspectorCommands(commands)
.expectMessages((message) => 'Runtime.consoleAPICalled' === message.method);
}

function breakOnLine(session) {
console.log('[test]', 'Breaking in the code');
const commands = [
{ 'method': 'Debugger.setBreakpointByUrl',
'params': { 'lineNumber': 9,
'url': script,
'columnNumber': 0,
'condition': ''
}
},
{ 'method': 'Runtime.evaluate',
'params': { 'expression': 'sum()',
'objectGroup': 'console',
'includeCommandLineAPI': true,
'silent': false,
'contextId': 1,
'returnByValue': false,
'generatePreview': true,
'userGesture': true,
'awaitPromise': false
}
}
];
helper.markMessageNoResponse(commands[1]);
session
.sendInspectorCommands(commands)
.expectMessages(setupExpectBreakOnLine(9, script, session));
}

function stepOverConsoleStatement(session) {
console.log('[test]', 'Step over console statement and test output');
session
.sendInspectorCommands({ 'method': 'Debugger.stepOver' })
.expectMessages(setupExpectConsoleOutputAndBreak('log', [0, 3]));
}

function testWaitsForFrontendDisconnect(session, harness) {
console.log('[test]', 'Verify node waits for the frontend to disconnect');
session.sendInspectorCommands({ 'method': 'Debugger.resume'})
.expectMessages(setupExpectContextDestroyed(1))
.expectStderrOutput('Waiting for the debugger to disconnect...')
.disconnect(true);
}

function runTests(harness) {
harness
.runFrontendSession([
setupDebugger,
breakOnLine,
stepOverConsoleStatement,
testWaitsForFrontendDisconnect
]).expectShutDown(0);
}

helper.startNodeForInspectorTest(runTests,
['--inspect'],
undefined,
script);

0 comments on commit 376ac5f

Please sign in to comment.