Skip to content

Commit

Permalink
inspector: patch C++ debug options instead of process._breakFirstLine
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
joyeecheung authored and targos committed Mar 30, 2019
1 parent 5e64acd commit 1b45704
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 11 deletions.
8 changes: 8 additions & 0 deletions src/env-inl.h
Expand Up @@ -684,6 +684,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
Expand Up @@ -887,6 +887,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 @@ -1104,6 +1107,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_;
Expand Down
16 changes: 5 additions & 11 deletions src/inspector_agent.cc
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
Expand Up @@ -553,6 +553,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
Expand Up @@ -85,6 +85,12 @@ class DebugOptions : public Options {
return deprecated_debug && !inspector_enabled;
}

// 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

0 comments on commit 1b45704

Please sign in to comment.