From ed6b456969a4c93b6bd712979d170e22dd033f3a Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 12 Mar 2019 04:28:51 +0800 Subject: [PATCH 1/2] 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. --- src/env-inl.h | 8 ++++++++ src/env.h | 5 +++++ src/inspector_agent.cc | 16 +++++----------- src/node_options.cc | 3 +++ src/node_options.h | 6 ++++++ 5 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index 3426393901966e..1023edfd2ee299 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -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; } diff --git a/src/env.h b/src/env.h index 62ce57b0bc37ed..872112aeb9e12b 100644 --- a/src/env.h +++ b/src/env.h @@ -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; @@ -1073,6 +1076,8 @@ class Environment { std::unordered_map 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_; diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 2376ae50044f0e..552c3bc7198f5f 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -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::ReadOnly | v8::DontEnum)) - .FromJust(); + CHECK(!parent_env_->has_serialized_options()); + debug_options_.EnableBreakFirstLine(); + parent_env_->options()->get_debug_options()->EnableBreakFirstLine(); client_->waitForFrontend(); } return true; diff --git a/src/node_options.cc b/src/node_options.cc index 85f0d447109d10..89c07e7049b9fe 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -569,6 +569,9 @@ void GetOptions(const FunctionCallbackInfo& args) { return env->ThrowError( "Should not query options before bootstrapping is done"); } + // Only allows serialization once. + CHECK(!env->has_serialized_options()); + env->set_has_serialized_options(true); Isolate* isolate = env->isolate(); Local context = env->context(); diff --git a/src/node_options.h b/src/node_options.h index a49425388c3053..e8fb00209464bd 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -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; } From d8f3ffe151a52fc6a32c39da7f5ef1a113e65a7a Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 13 Mar 2019 00:48:00 +0800 Subject: [PATCH 2/2] fixup! inspector: patch C++ debug options instead of process._breakFirstLine --- src/node_options.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/node_options.cc b/src/node_options.cc index 89c07e7049b9fe..fc249bff547591 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -569,8 +569,6 @@ void GetOptions(const FunctionCallbackInfo& args) { return env->ThrowError( "Should not query options before bootstrapping is done"); } - // Only allows serialization once. - CHECK(!env->has_serialized_options()); env->set_has_serialized_options(true); Isolate* isolate = env->isolate();