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

src: refactor options parsing #22392

Closed
wants to merge 4 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
5 changes: 3 additions & 2 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -344,14 +344,14 @@
'src/node_config.cc',
'src/node_constants.cc',
'src/node_contextify.cc',
'src/node_debug_options.cc',
'src/node_domain.cc',
'src/node_encoding.cc',
'src/node_errors.h',
'src/node_file.cc',
'src/node_http2.cc',
'src/node_http_parser.cc',
'src/node_messaging.cc',
'src/node_options.cc',
'src/node_os.cc',
'src/node_platform.cc',
'src/node_perf.cc',
Expand Down Expand Up @@ -408,14 +408,15 @@
'src/node_code_cache.h',
'src/node_constants.h',
'src/node_contextify.h',
'src/node_debug_options.h',
'src/node_file.h',
'src/node_http2.h',
'src/node_http2_state.h',
'src/node_internals.h',
'src/node_javascript.h',
'src/node_messaging.h',
'src/node_mutex.h',
'src/node_options.h',
'src/node_options-inl.h',
'src/node_perf.h',
'src/node_perf_common.h',
'src/node_persistent.h',
Expand Down
8 changes: 8 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,14 @@ Environment::file_handle_read_wrap_freelist() {
return file_handle_read_wrap_freelist_;
}

inline std::shared_ptr<EnvironmentOptions> Environment::options() {
return options_;
}

inline std::shared_ptr<PerIsolateOptions> IsolateData::options() {
return options_;
}

void Environment::CreateImmediate(native_immediate_callback cb,
void* data,
v8::Local<v8::Object> obj,
Expand Down
26 changes: 18 additions & 8 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ IsolateData::IsolateData(Isolate* isolate,
if (platform_ != nullptr)
platform_->RegisterIsolate(this, event_loop);

options_.reset(new PerIsolateOptions(*per_process_opts->per_isolate));

// Create string and private symbol properties as internalized one byte
// strings after the platform is properly initialized.
//
Expand Down Expand Up @@ -136,9 +138,6 @@ Environment::Environment(IsolateData* isolate_data,
makecallback_cntr_(0),
should_abort_on_uncaught_toggle_(isolate_, 1),
trace_category_state_(isolate_, kTraceCategoryCount),
#if HAVE_INSPECTOR
inspector_agent_(new inspector::Agent(this)),
#endif
http_parser_buffer_(nullptr),
fs_stats_field_array_(isolate_, kFsStatsFieldsLength * 2),
fs_stats_field_bigint_array_(isolate_, kFsStatsFieldsLength * 2),
Expand All @@ -148,6 +147,19 @@ Environment::Environment(IsolateData* isolate_data,
v8::Context::Scope context_scope(context);
set_as_external(v8::External::New(isolate(), this));

// We create new copies of the per-Environment option sets, so that it is
// easier to modify them after Environment creation. The defaults are
// part of the per-Isolate option set, for which in turn the defaults are
// part of the per-process option set.
options_.reset(new EnvironmentOptions(*isolate_data->options()->per_env));
options_->debug_options.reset(new DebugOptions(*options_->debug_options));

#if HAVE_INSPECTOR
// We can only create the inspector agent after having cloned the options.
inspector_agent_ =
std::unique_ptr<inspector::Agent>(new inspector::Agent(this));
#endif

AssignToContext(context, ContextInfo(""));

if (tracing_agent_writer_ != nullptr) {
Expand Down Expand Up @@ -211,10 +223,8 @@ Environment::~Environment() {
delete[] http_parser_buffer_;
}

void Environment::Start(int argc,
const char* const* argv,
int exec_argc,
const char* const* exec_argv,
void Environment::Start(const std::vector<std::string>& args,
const std::vector<std::string>& exec_args,
bool start_profiler_idle_notifier) {
HandleScope handle_scope(isolate());
Context::Scope context_scope(context());
Expand Down Expand Up @@ -260,7 +270,7 @@ void Environment::Start(int argc,
process_template->GetFunction()->NewInstance(context()).ToLocalChecked();
set_process_object(process_object);

SetupProcessObject(this, argc, argv, exec_argc, exec_argv);
SetupProcessObject(this, args, exec_args);

static uv_once_t init_once = UV_ONCE_INIT;
uv_once(&init_once, InitThreadLocalOnce);
Expand Down
13 changes: 9 additions & 4 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "uv.h"
#include "v8.h"
#include "node.h"
#include "node_options.h"
#include "node_http2_state.h"

#include <list>
Expand Down Expand Up @@ -361,6 +362,7 @@ class IsolateData {
inline uv_loop_t* event_loop() const;
inline uint32_t* zero_fill_field() const;
inline MultiIsolatePlatform* platform() const;
inline std::shared_ptr<PerIsolateOptions> options();

#define VP(PropertyName, StringValue) V(v8::Private, PropertyName)
#define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
Expand Down Expand Up @@ -394,6 +396,7 @@ class IsolateData {
uv_loop_t* const event_loop_;
uint32_t* const zero_fill_field_;
MultiIsolatePlatform* platform_;
std::shared_ptr<PerIsolateOptions> options_;

DISALLOW_COPY_AND_ASSIGN(IsolateData);
};
Expand Down Expand Up @@ -578,10 +581,8 @@ class Environment {
tracing::AgentWriterHandle* tracing_agent_writer);
~Environment();

void Start(int argc,
const char* const* argv,
int exec_argc,
const char* const* exec_argv,
void Start(const std::vector<std::string>& args,
const std::vector<std::string>& exec_args,
bool start_profiler_idle_notifier);

typedef void (*HandleCleanupCb)(Environment* env,
Expand Down Expand Up @@ -878,6 +879,8 @@ class Environment {
v8::EmbedderGraph* graph,
void* data);

inline std::shared_ptr<EnvironmentOptions> options();

private:
inline void CreateImmediate(native_immediate_callback cb,
void* data,
Expand Down Expand Up @@ -908,6 +911,8 @@ class Environment {
size_t makecallback_cntr_;
std::vector<double> destroy_async_id_list_;

std::shared_ptr<EnvironmentOptions> options_;

AliasedBuffer<uint32_t, v8::Uint32Array> should_abort_on_uncaught_toggle_;
int should_not_abort_scope_counter_ = 0;

Expand Down
13 changes: 8 additions & 5 deletions src/inspector_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -608,11 +608,14 @@ class NodeInspectorClient : public V8InspectorClient {
std::unique_ptr<MainThreadInterface> interface_;
};

Agent::Agent(Environment* env) : parent_env_(env) {}
Agent::Agent(Environment* env)
: parent_env_(env),
debug_options_(env->options()->debug_options) {}

Agent::~Agent() = default;

bool Agent::Start(const std::string& path, const DebugOptions& options) {
bool Agent::Start(const std::string& path,
std::shared_ptr<DebugOptions> options) {
path_ = path;
debug_options_ = options;
client_ = std::make_shared<NodeInspectorClient>(parent_env_);
Expand All @@ -626,8 +629,8 @@ bool Agent::Start(const std::string& path, const DebugOptions& options) {
StartDebugSignalHandler();
}

bool wait_for_connect = options.wait_for_connect();
if (!options.inspector_enabled() || !StartIoThread()) {
bool wait_for_connect = options->wait_for_connect();
if (!options->inspector_enabled || !StartIoThread()) {
return false;
}
if (wait_for_connect) {
Expand Down Expand Up @@ -789,7 +792,7 @@ void Agent::ContextCreated(Local<Context> context, const ContextInfo& info) {
}

bool Agent::WillWaitForConnect() {
return debug_options_.wait_for_connect();
return debug_options_->wait_for_connect();
}

bool Agent::IsActive() {
Expand Down
8 changes: 4 additions & 4 deletions src/inspector_agent.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#error("This header can only be used when inspector is enabled")
#endif

#include "node_debug_options.h"
#include "node_options.h"
#include "node_persistent.h"
#include "v8.h"

Expand Down Expand Up @@ -45,7 +45,7 @@ class Agent {
~Agent();

// Create client_, may create io_ if option enabled
bool Start(const std::string& path, const DebugOptions& options);
bool Start(const std::string& path, std::shared_ptr<DebugOptions> options);
// Stop and destroy io_
void Stop();

Expand Down Expand Up @@ -96,7 +96,7 @@ class Agent {
// Calls StartIoThread() from off the main thread.
void RequestIoThreadStart();

DebugOptions& options() { return debug_options_; }
std::shared_ptr<DebugOptions> options() { return debug_options_; }
void ContextCreated(v8::Local<v8::Context> context, const ContextInfo& info);

private:
Expand All @@ -109,7 +109,7 @@ class Agent {
// Interface for transports, e.g. WebSocket server
std::unique_ptr<InspectorIo> io_;
std::string path_;
DebugOptions debug_options_;
std::shared_ptr<DebugOptions> debug_options_;

bool pending_enable_async_hook_ = false;
bool pending_disable_async_hook_ = false;
Expand Down
7 changes: 4 additions & 3 deletions src/inspector_io.cc
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ class InspectorIoDelegate: public node::inspector::SocketServerDelegate {
std::unique_ptr<InspectorIo> InspectorIo::Start(
std::shared_ptr<MainThreadHandle> main_thread,
const std::string& path,
const DebugOptions& options) {
std::shared_ptr<DebugOptions> options) {
auto io = std::unique_ptr<InspectorIo>(
new InspectorIo(main_thread, path, options));
if (io->request_queue_->Expired()) { // Thread is not running
Expand All @@ -253,7 +253,7 @@ std::unique_ptr<InspectorIo> InspectorIo::Start(

InspectorIo::InspectorIo(std::shared_ptr<MainThreadHandle> main_thread,
const std::string& path,
const DebugOptions& options)
std::shared_ptr<DebugOptions> options)
: main_thread_(main_thread), options_(options),
thread_(), script_name_(path), id_(GenerateID()) {
Mutex::ScopedLock scoped_lock(thread_start_lock_);
Expand Down Expand Up @@ -288,7 +288,8 @@ void InspectorIo::ThreadMain() {
new InspectorIoDelegate(queue, main_thread_, id_,
script_path, script_name_));
InspectorSocketServer server(std::move(delegate), &loop,
options_.host_name(), options_.port());
options_->host().c_str(),
options_->port());
request_queue_ = queue->handle();
// Its lifetime is now that of the server delegate
queue.reset();
Expand Down
10 changes: 5 additions & 5 deletions src/inspector_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
#define SRC_INSPECTOR_IO_H_

#include "inspector_socket_server.h"
#include "node_debug_options.h"
#include "node_mutex.h"
#include "uv.h"

Expand Down Expand Up @@ -46,19 +45,20 @@ class InspectorIo {
// Returns empty pointer if thread was not started
static std::unique_ptr<InspectorIo> Start(
std::shared_ptr<MainThreadHandle> main_thread, const std::string& path,
const DebugOptions& options);
std::shared_ptr<DebugOptions> options);

// Will block till the transport thread shuts down
~InspectorIo();

void StopAcceptingNewConnections();
std::string host() const { return options_.host_name(); }
const std::string& host() const { return options_->host(); }
int port() const { return port_; }
std::vector<std::string> GetTargetIds() const;

private:
InspectorIo(std::shared_ptr<MainThreadHandle> handle,
const std::string& path, const DebugOptions& options);
const std::string& path,
std::shared_ptr<DebugOptions> options);

// Wrapper for agent->ThreadMain()
static void ThreadMain(void* agent);
Expand All @@ -72,7 +72,7 @@ class InspectorIo {
// Used to post on a frontend interface thread, lives while the server is
// running
std::shared_ptr<RequestQueue> request_queue_;
const DebugOptions options_;
std::shared_ptr<DebugOptions> options_;

// The IO thread runs its own uv_loop to implement the TCP server off
// the main thread.
Expand Down
4 changes: 2 additions & 2 deletions src/inspector_js_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -242,12 +242,12 @@ void Open(const FunctionCallbackInfo<Value>& args) {

if (args.Length() > 0 && args[0]->IsUint32()) {
uint32_t port = args[0]->Uint32Value();
agent->options().set_port(static_cast<int>(port));
agent->options()->host_port.port = port;
}

if (args.Length() > 1 && args[1]->IsString()) {
Utf8Value host(env->isolate(), args[1].As<String>());
agent->options().set_host_name(*host);
agent->options()->host_port.host_name = *host;
}

if (args.Length() > 2 && args[2]->IsBoolean()) {
Expand Down
Loading