Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 845bcfa

Browse filesBrowse files
joyeecheungaddaleax
authored andcommitted
src: simplify inspector initialization in node::Start()
Remove the `StartInspector` and `InspectorStarted` abstraction out of `v8_platform`, and error out early and directly in the option parser if Node is configured with NODE_USE_V8_PLATFORM and inspector enabled but the user still tries to use inspector options. PR-URL: #25612 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent 7d68223 commit 845bcfa
Copy full SHA for 845bcfa

File tree

Expand file treeCollapse file tree

3 files changed

+26
-38
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+26
-38
lines changed
Open diff view settings
Collapse file

‎src/node.cc‎

Copy file name to clipboardExpand all lines: src/node.cc
+15-38Lines changed: 15 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -262,23 +262,6 @@ static struct {
262262
platform_->CancelPendingDelayedTasks(isolate);
263263
}
264264

265-
#if HAVE_INSPECTOR
266-
bool StartInspector(Environment* env, const char* script_path) {
267-
// Inspector agent can't fail to start, but if it was configured to listen
268-
// right away on the websocket port and fails to bind/etc, this will return
269-
// false.
270-
return env->inspector_agent()->Start(
271-
script_path == nullptr ? "" : script_path,
272-
env->options()->debug_options(),
273-
env->inspector_host_port(),
274-
true);
275-
}
276-
277-
bool InspectorStarted(Environment* env) {
278-
return env->inspector_agent()->IsListening();
279-
}
280-
#endif // HAVE_INSPECTOR
281-
282265
void StartTracingAgent() {
283266
if (per_process::cli_options->trace_event_categories.empty()) {
284267
tracing_file_writer_ = tracing_agent_->DefaultHandle();
@@ -317,10 +300,6 @@ static struct {
317300
void Dispose() {}
318301
void DrainVMTasks(Isolate* isolate) {}
319302
void CancelVMTasks(Isolate* isolate) {}
320-
bool StartInspector(Environment* env, const char* script_path) {
321-
env->ThrowError("Node compiled with NODE_USE_V8_PLATFORM=0");
322-
return true;
323-
}
324303

325304
void StartTracingAgent() {
326305
if (!trace_enabled_categories.empty()) {
@@ -338,12 +317,6 @@ static struct {
338317
return nullptr;
339318
}
340319
#endif // !NODE_USE_V8_PLATFORM
341-
342-
#if !NODE_USE_V8_PLATFORM || !HAVE_INSPECTOR
343-
bool InspectorStarted(Environment* env) {
344-
return false;
345-
}
346-
#endif // !NODE_USE_V8_PLATFORM || !HAVE_INSPECTOR
347320
} v8_platform;
348321

349322
tracing::AgentWriterHandle* GetTracingAgentWriter() {
@@ -774,13 +747,6 @@ void StartExecution(Environment* env, const char* main_script_id) {
774747
env->context(), Undefined(env->isolate()), arraysize(argv), argv));
775748
}
776749

777-
static void StartInspector(Environment* env, const char* path) {
778-
#if HAVE_INSPECTOR
779-
CHECK(!env->inspector_agent()->IsListening());
780-
v8_platform.StartInspector(env, path);
781-
#endif // HAVE_INSPECTOR
782-
}
783-
784750

785751
#ifdef __POSIX__
786752
void RegisterSignalHandler(int signal,
@@ -1280,13 +1246,24 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
12801246
Environment env(isolate_data, context);
12811247
env.Start(args, exec_args, per_process::v8_is_profiling);
12821248

1283-
const char* path = args.size() > 1 ? args[1].c_str() : nullptr;
1284-
StartInspector(&env, path);
1285-
1249+
#if HAVE_INSPECTOR && NODE_USE_V8_PLATFORM
1250+
CHECK(!env.inspector_agent()->IsListening());
1251+
// Inspector agent can't fail to start, but if it was configured to listen
1252+
// right away on the websocket port and fails to bind/etc, this will return
1253+
// false.
1254+
env.inspector_agent()->Start(args.size() > 1 ? args[1].c_str() : "",
1255+
env.options()->debug_options(),
1256+
env.inspector_host_port(),
1257+
true);
12861258
if (env.options()->debug_options().inspector_enabled &&
1287-
!v8_platform.InspectorStarted(&env)) {
1259+
!env.inspector_agent()->IsListening()) {
12881260
return 12; // Signal internal error.
12891261
}
1262+
#else
1263+
// inspector_enabled can't be true if !HAVE_INSPECTOR or !NODE_USE_V8_PLATFORM
1264+
// - the option parser should not allow that.
1265+
CHECK(!env.options()->debug_options().inspector_enabled);
1266+
#endif // HAVE_INSPECTOR && NODE_USE_V8_PLATFORM
12901267

12911268
{
12921269
Environment::AsyncCallbackScope callback_scope(&env);
Collapse file

‎src/node_options.cc‎

Copy file name to clipboardExpand all lines: src/node_options.cc
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,15 @@ Mutex cli_options_mutex;
2323
std::shared_ptr<PerProcessOptions> cli_options{new PerProcessOptions()};
2424
} // namespace per_process
2525

26+
void DebugOptions::CheckOptions(std::vector<std::string>* errors) {
27+
#if !NODE_USE_V8_PLATFORM
28+
if (inspector_enabled) {
29+
errors->push_back("Inspector is not available when Node is compiled "
30+
"--without-v8-platform");
31+
}
32+
#endif
33+
}
34+
2635
void PerProcessOptions::CheckOptions(std::vector<std::string>* errors) {
2736
#if HAVE_OPENSSL
2837
if (use_openssl_ca && use_bundled_ca) {
Collapse file

‎src/node_options.h‎

Copy file name to clipboardExpand all lines: src/node_options.h
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ class DebugOptions : public Options {
8888
bool wait_for_connect() const {
8989
return break_first_line || break_node_first_line;
9090
}
91+
92+
void CheckOptions(std::vector<std::string>* errors);
9193
};
9294

9395
class EnvironmentOptions : public Options {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.