Skip to content

Commit 74a3c97

Browse files
alexkozyalexeykozy
authored andcommitted
inspector: added NodeRuntime domain
Historically Node process sends Runtime.executionContextDestroyed with main context as argument when it is finished. This approach has some disadvantages. V8 prevents running some protocol command on destroyed contexts, e.g. Runtime.evaluate will return an error or Debugger.enable won't return a list of scripts. Both command might be useful for different tools, e.g. tool runs Profiler.startPreciseCoverage and at the end of node process it would like to get list of all scripts to match data to source code. Or some tooling frontend would like to provide capabilities to run commands in console when node process is finished to allow user to inspect state of the program at exit. This PR adds new domain: NodeRuntime. When this domain is enabled by at least one client, node will send NodeRuntime.waitingForDebuggerToDisconnect event instead of Runtime.executionContextDestroyed. Based on this signal any protocol client can capture all required information and then disconnect its session.
1 parent 153c101 commit 74a3c97

7 files changed

+144
-1
lines changed

src/inspector/node_inspector.gypi

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
'<(SHARED_INTERMEDIATE_DIR)/src/node/inspector/protocol/NodeWorker.h',
1010
'<(SHARED_INTERMEDIATE_DIR)/src/node/inspector/protocol/NodeTracing.cpp',
1111
'<(SHARED_INTERMEDIATE_DIR)/src/node/inspector/protocol/NodeTracing.h',
12+
'<(SHARED_INTERMEDIATE_DIR)/src/node/inspector/protocol/NodeRuntime.cpp',
13+
'<(SHARED_INTERMEDIATE_DIR)/src/node/inspector/protocol/NodeRuntime.h',
1214
],
1315
'node_protocol_files': [
1416
'<(protocol_tool_path)/lib/Allocator_h.template',
@@ -55,6 +57,8 @@
5557
'../../src/inspector/main_thread_interface.h',
5658
'../../src/inspector/node_string.cc',
5759
'../../src/inspector/node_string.h',
60+
'../../src/inspector/runtime_agent.cc',
61+
'../../src/inspector/runtime_agent.h',
5862
'../../src/inspector/tracing_agent.cc',
5963
'../../src/inspector/tracing_agent.h',
6064
'../../src/inspector/worker_agent.cc',

src/inspector/node_protocol.pdl

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,3 +92,12 @@ experimental domain NodeWorker
9292
# Identifier of a session which sends a message.
9393
SessionID sessionId
9494
string message
95+
96+
experimental domain NodeRuntime
97+
command enable
98+
command disable
99+
100+
# Event is issued when Node process is finished and waiting for debugger to disconnect.
101+
# When domain is enabled client should listen for this event instead of `Runtime.executionContextDestroyed`
102+
# to check wether node process is finished.
103+
event waitingForDebuggerToDisconnect

src/inspector/runtime_agent.cc

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
#include "runtime_agent.h"
2+
3+
#include "env-inl.h"
4+
#include "inspector_agent.h"
5+
6+
namespace node {
7+
namespace inspector {
8+
namespace protocol {
9+
10+
RuntimeAgent::RuntimeAgent(Environment* env)
11+
: enabled_(false)
12+
, env_(env) {}
13+
14+
void RuntimeAgent::Wire(UberDispatcher* dispatcher) {
15+
frontend_.reset(new NodeRuntime::Frontend(dispatcher->channel()));
16+
NodeRuntime::Dispatcher::wire(dispatcher, this);
17+
}
18+
19+
DispatchResponse RuntimeAgent::enable() {
20+
if (!env_->owns_process_state()) {
21+
return DispatchResponse::Error(
22+
"NodeRuntime domain can only be used through main thread sessions");
23+
}
24+
enabled_ = true;
25+
return DispatchResponse::OK();
26+
}
27+
28+
DispatchResponse RuntimeAgent::disable() {
29+
if (!env_->owns_process_state()) {
30+
return DispatchResponse::Error(
31+
"NodeRuntime domain can only be used through main thread sessions");
32+
}
33+
enabled_ = false;
34+
return DispatchResponse::OK();
35+
}
36+
37+
bool RuntimeAgent::reportWaitingForDebuggerToDisconnect() {
38+
if (enabled_) frontend_->waitingForDebuggerToDisconnect();
39+
return enabled_;
40+
}
41+
} // namespace protocol
42+
} // namespace inspector
43+
} // namespace node

src/inspector/runtime_agent.h

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
#ifndef SRC_INSPECTOR_RUNTIME_AGENT_H_
2+
#define SRC_INSPECTOR_RUNTIME_AGENT_H_
3+
4+
#include "node/inspector/protocol/NodeRuntime.h"
5+
#include "v8.h"
6+
7+
namespace node {
8+
class Environment;
9+
10+
namespace inspector {
11+
namespace protocol {
12+
13+
class RuntimeAgent : public NodeRuntime::Backend {
14+
public:
15+
explicit RuntimeAgent(Environment* env);
16+
17+
void Wire(UberDispatcher* dispatcher);
18+
19+
DispatchResponse enable() override;
20+
DispatchResponse disable() override;
21+
22+
bool reportWaitingForDebuggerToDisconnect();
23+
24+
private:
25+
std::shared_ptr<NodeRuntime::Frontend> frontend_;
26+
bool enabled_;
27+
Environment* env_;
28+
};
29+
} // namespace protocol
30+
} // namespace inspector
31+
} // namespace node
32+
33+
#endif // SRC_INSPECTOR_RUNTIME_AGENT_H_

src/inspector_agent.cc

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#include "inspector/main_thread_interface.h"
44
#include "inspector/node_string.h"
5+
#include "inspector/runtime_agent.h"
56
#include "inspector/tracing_agent.h"
67
#include "inspector/worker_agent.h"
78
#include "inspector/worker_inspector.h"
@@ -228,13 +229,17 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel,
228229
tracing_agent_->Wire(node_dispatcher_.get());
229230
worker_agent_ = std::make_unique<protocol::WorkerAgent>(worker_manager);
230231
worker_agent_->Wire(node_dispatcher_.get());
232+
runtime_agent_.reset(new protocol::RuntimeAgent(env));
233+
runtime_agent_->Wire(node_dispatcher_.get());
231234
}
232235

233236
~ChannelImpl() override {
234237
tracing_agent_->disable();
235238
tracing_agent_.reset(); // Dispose before the dispatchers
236239
worker_agent_->disable();
237240
worker_agent_.reset(); // Dispose before the dispatchers
241+
runtime_agent_->disable();
242+
runtime_agent_.reset(); // Dispose before the dispatchers
238243
}
239244

240245
std::string dispatchProtocolMessage(const StringView& message) {
@@ -264,6 +269,10 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel,
264269
return prevent_shutdown_;
265270
}
266271

272+
bool reportWaitingForDebuggerToDisconnect() {
273+
return runtime_agent_->reportWaitingForDebuggerToDisconnect();
274+
}
275+
267276
private:
268277
void sendResponse(
269278
int callId,
@@ -303,6 +312,7 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel,
303312
DCHECK(false);
304313
}
305314

315+
std::unique_ptr<protocol::RuntimeAgent> runtime_agent_;
306316
std::unique_ptr<protocol::TracingAgent> tracing_agent_;
307317
std::unique_ptr<protocol::WorkerAgent> worker_agent_;
308318
std::unique_ptr<InspectorSessionDelegate> delegate_;
@@ -610,6 +620,14 @@ class NodeInspectorClient : public V8InspectorClient {
610620
return false;
611621
}
612622

623+
bool reportWaitingForDebuggerToDisconnect() {
624+
for (const auto& id_channel : channels_) {
625+
if (id_channel.second->reportWaitingForDebuggerToDisconnect())
626+
return true;
627+
}
628+
return false;
629+
}
630+
613631
std::shared_ptr<MainThreadHandle> getThreadHandle() {
614632
if (interface_ == nullptr) {
615633
interface_.reset(new MainThreadInterface(
@@ -779,7 +797,8 @@ void Agent::WaitForDisconnect() {
779797
}
780798
// TODO(addaleax): Maybe this should use an at-exit hook for the Environment
781799
// or something similar?
782-
client_->contextDestroyed(parent_env_->context());
800+
if (!client_->reportWaitingForDebuggerToDisconnect())
801+
client_->contextDestroyed(parent_env_->context());
783802
if (io_ != nullptr) {
784803
io_->StopAcceptingNewConnections();
785804
client_->waitForIoShutdown();

test/common/inspector-helper.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,10 @@ class InspectorSession {
191191
}
192192
}
193193

194+
unprocessedNotifications() {
195+
return this._unprocessedNotifications;
196+
}
197+
194198
_sendMessage(message) {
195199
const msg = JSON.parse(JSON.stringify(message)); // Clone!
196200
msg.id = this._nextId++;
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Flags: --expose-internals
2+
'use strict';
3+
const common = require('../common');
4+
5+
common.skipIfInspectorDisabled();
6+
7+
const assert = require('assert');
8+
const { NodeInstance } = require('../common/inspector-helper.js');
9+
10+
async function runTest() {
11+
const child = new NodeInstance(['--inspect-brk=0', '-e', 'process.exit(55)']);
12+
const session = await child.connectInspectorSession();
13+
await session.send([
14+
{ method: 'Runtime.enable' },
15+
{ method: 'NodeRuntime.enable' },
16+
{ method: 'Runtime.runIfWaitingForDebugger' }]);
17+
await session.waitForNotification((notification) => {
18+
return notification.method === 'NodeRuntime.waitingForDebuggerToDisconnect';
19+
});
20+
const receivedExecutionContextDestroyed = session.unprocessedNotifications().some((notification) => {
21+
return notification.method === 'Runtime.executionContextDestroyed' &&
22+
notification.params.executionContextId === 1;
23+
});
24+
if (receivedExecutionContextDestroyed) {
25+
assert.fail(`When NodeRuntime enabled, Runtime.executionContextDestroyed should not be sent`);
26+
}
27+
await session.disconnect();
28+
assert.strictEqual((await child.expectShutdown()).exitCode, 55);
29+
}
30+
31+
runTest();

0 commit comments

Comments
 (0)