Skip to content

Commit 97a919b

Browse files
committed
inspector: patch C++ debug options instead of process._breakFirstLine
Instead of patching process._breakFirstLine to inform the JS land to wait for the debugger, check that the JS land has not yet serialized the options and then patch the debug options from C++. The changes will be carried into JS later during option serialization. PR-URL: #26602 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent a91d36f commit 97a919b

File tree

5 files changed

+25
-11
lines changed

5 files changed

+25
-11
lines changed

src/env-inl.h

+8
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,14 @@ inline void Environment::set_has_run_bootstrapping_code(bool value) {
675675
has_run_bootstrapping_code_ = value;
676676
}
677677

678+
inline bool Environment::has_serialized_options() const {
679+
return has_serialized_options_;
680+
}
681+
682+
inline void Environment::set_has_serialized_options(bool value) {
683+
has_serialized_options_ = value;
684+
}
685+
678686
inline bool Environment::is_main_thread() const {
679687
return flags_ & kIsMainThread;
680688
}

src/env.h

+5
Original file line numberDiff line numberDiff line change
@@ -862,6 +862,9 @@ class Environment {
862862
inline bool has_run_bootstrapping_code() const;
863863
inline void set_has_run_bootstrapping_code(bool has_run_bootstrapping_code);
864864

865+
inline bool has_serialized_options() const;
866+
inline void set_has_serialized_options(bool has_serialized_options);
867+
865868
static uint64_t AllocateThreadId();
866869
static constexpr uint64_t kNoThreadId = -1;
867870

@@ -1106,6 +1109,8 @@ class Environment {
11061109
std::unordered_map<std::string, uint64_t> performance_marks_;
11071110

11081111
bool has_run_bootstrapping_code_ = false;
1112+
bool has_serialized_options_ = false;
1113+
11091114
bool can_call_into_js_ = true;
11101115
Flags flags_;
11111116
uint64_t thread_id_;

src/inspector_agent.cc

+5-11
Original file line numberDiff line numberDiff line change
@@ -728,18 +728,12 @@ bool Agent::Start(const std::string& path,
728728
return false;
729729
}
730730

731-
// TODO(joyeecheung): we should not be using process as a global object
732-
// to transport --inspect-brk. Instead, the JS land can get this through
733-
// require('internal/options') since it should be set once CLI parsing
734-
// is done.
731+
// Patch the debug options to implement waitForDebuggerOnStart for
732+
// the NodeWorker.enable method.
735733
if (wait_for_connect) {
736-
HandleScope scope(parent_env_->isolate());
737-
parent_env_->process_object()->DefineOwnProperty(
738-
parent_env_->context(),
739-
FIXED_ONE_BYTE_STRING(parent_env_->isolate(), "_breakFirstLine"),
740-
True(parent_env_->isolate()),
741-
static_cast<v8::PropertyAttribute>(v8::ReadOnly | v8::DontEnum))
742-
.FromJust();
734+
CHECK(!parent_env_->has_serialized_options());
735+
debug_options_.EnableBreakFirstLine();
736+
parent_env_->options()->get_debug_options()->EnableBreakFirstLine();
743737
client_->waitForFrontend();
744738
}
745739
return true;

src/node_options.cc

+1
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,7 @@ void GetOptions(const FunctionCallbackInfo<Value>& args) {
571571
return env->ThrowError(
572572
"Should not query options before bootstrapping is done");
573573
}
574+
env->set_has_serialized_options(true);
574575

575576
Isolate* isolate = env->isolate();
576577
Local<Context> context = env->context();

src/node_options.h

+6
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,12 @@ class DebugOptions : public Options {
7575

7676
HostPort host_port{"127.0.0.1", kDefaultInspectorPort};
7777

78+
// Used to patch the options as if --inspect-brk is passed.
79+
void EnableBreakFirstLine() {
80+
inspector_enabled = true;
81+
break_first_line = true;
82+
}
83+
7884
bool wait_for_connect() const {
7985
return break_first_line || break_node_first_line;
8086
}

0 commit comments

Comments
 (0)