Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inspector: patch C++ debug options instead of process._breakFirstLine #26602

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,14 @@ inline void Environment::set_has_run_bootstrapping_code(bool value) {
has_run_bootstrapping_code_ = value;
}

inline bool Environment::has_serialized_options() const {
return has_serialized_options_;
}

inline void Environment::set_has_serialized_options(bool value) {
has_serialized_options_ = value;
}

inline bool Environment::is_main_thread() const {
return flags_ & kIsMainThread;
}
Expand Down
5 changes: 5 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,9 @@ class Environment {
inline bool has_run_bootstrapping_code() const;
inline void set_has_run_bootstrapping_code(bool has_run_bootstrapping_code);

inline bool has_serialized_options() const;
inline void set_has_serialized_options(bool has_serialized_options);

static uint64_t AllocateThreadId();
static constexpr uint64_t kNoThreadId = -1;

Expand Down Expand Up @@ -1073,6 +1076,8 @@ class Environment {
std::unordered_map<std::string, uint64_t> performance_marks_;

bool has_run_bootstrapping_code_ = false;
bool has_serialized_options_ = false;

bool can_call_into_js_ = true;
Flags flags_;
uint64_t thread_id_;
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
16 changes: 5 additions & 11 deletions src/inspector_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -728,18 +728,12 @@ bool Agent::Start(const std::string& path,
return false;
}

// TODO(joyeecheung): we should not be using process as a global object
// to transport --inspect-brk. Instead, the JS land can get this through
// require('internal/options') since it should be set once CLI parsing
// is done.
// Patch the debug options to implement waitForDebuggerOnStart for
// the NodeWorker.enable method.
if (wait_for_connect) {
HandleScope scope(parent_env_->isolate());
parent_env_->process_object()->DefineOwnProperty(
parent_env_->context(),
FIXED_ONE_BYTE_STRING(parent_env_->isolate(), "_breakFirstLine"),
True(parent_env_->isolate()),
static_cast<v8::PropertyAttribute>(v8::ReadOnly | v8::DontEnum))
.FromJust();
CHECK(!parent_env_->has_serialized_options());
debug_options_.EnableBreakFirstLine();
parent_env_->options()->get_debug_options()->EnableBreakFirstLine();
client_->waitForFrontend();
}
return true;
Expand Down
1 change: 1 addition & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,7 @@ void GetOptions(const FunctionCallbackInfo<Value>& args) {
return env->ThrowError(
"Should not query options before bootstrapping is done");
}
env->set_has_serialized_options(true);

Isolate* isolate = env->isolate();
Local<Context> context = env->context();
Expand Down
6 changes: 6 additions & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ class DebugOptions : public Options {

HostPort host_port{"127.0.0.1", kDefaultInspectorPort};

// Used to patch the options as if --inspect-brk is passed.
void EnableBreakFirstLine() {
inspector_enabled = true;
break_first_line = true;
}

bool wait_for_connect() const {
return break_first_line || break_node_first_line;
}
Expand Down