From f02c844b915359edaadbc40b0513a382a5a31e8d Mon Sep 17 00:00:00 2001 From: Taylor Woll Date: Thu, 25 Oct 2018 20:34:34 -0700 Subject: [PATCH] build: fix static initialization ordering problem in OptionsParser code --- src/node.cc | 2 +- src/node_options.cc | 24 ++++++++---------------- src/node_options.h | 40 ++++++++++++++++++++++++++++++++-------- 3 files changed, 41 insertions(+), 25 deletions(-) diff --git a/src/node.cc b/src/node.cc index 393cae9b9b5..b102aa86b38 100644 --- a/src/node.cc +++ b/src/node.cc @@ -2513,7 +2513,7 @@ void ProcessArgv(std::vector* args, // TODO(addaleax): The mutex here should ideally be held during the // entire function, but that doesn't play well with the exit() calls below. Mutex::ScopedLock lock(per_process_opts_mutex); - options_parser::PerProcessOptionsParser::instance.Parse( + options_parser::PerProcessOptionsParser::GetInstance()->Parse( args, exec_args, &v8_args, diff --git a/src/node_options.cc b/src/node_options.cc index 4fdd0645932..32debd68e42 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -48,7 +48,7 @@ namespace options_parser { // doc/api/cli.md // TODO(addaleax): Make that unnecessary. -DebugOptionsParser::DebugOptionsParser() { +void DebugOptionsParser::Initialize() { #if HAVE_INSPECTOR AddOption("--inspect-port", "set host:port for inspector", @@ -82,9 +82,7 @@ DebugOptionsParser::DebugOptionsParser() { #endif } -DebugOptionsParser DebugOptionsParser::instance; - -EnvironmentOptionsParser::EnvironmentOptionsParser() { +void EnvironmentOptionsParser::Initialize() { AddOption("--experimental-modules", "experimental ES Module support and caching modules", &EnvironmentOptions::experimental_modules, @@ -191,13 +189,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { AddOption("--expose-http2", "", NoOp {}, kAllowedInEnvironment); AddOption("--expose_http2", "", NoOp {}, kAllowedInEnvironment); - Insert(&DebugOptionsParser::instance, + Insert(DebugOptionsParser::GetInstance(), &EnvironmentOptions::get_debug_options); } -EnvironmentOptionsParser EnvironmentOptionsParser::instance; - -PerIsolateOptionsParser::PerIsolateOptionsParser() { +void PerIsolateOptionsParser::Initialize() { AddOption("--track-heap-objects", "track heap object allocations for heap snapshots", &PerIsolateOptions::track_heap_objects, @@ -214,13 +210,11 @@ PerIsolateOptionsParser::PerIsolateOptionsParser() { AddOption("--perf-prof", "", V8Option{}, kAllowedInEnvironment); AddOption("--stack-trace-limit", "", V8Option{}, kAllowedInEnvironment); - Insert(&EnvironmentOptionsParser::instance, + Insert(EnvironmentOptionsParser::GetInstance(), &PerIsolateOptions::get_per_env_options); } -PerIsolateOptionsParser PerIsolateOptionsParser::instance; - -PerProcessOptionsParser::PerProcessOptionsParser() { +void PerProcessOptionsParser::Initialize() { AddOption("--title", "the process title to use on startup", &PerProcessOptions::title, @@ -347,12 +341,10 @@ PerProcessOptionsParser::PerProcessOptionsParser() { &PerProcessOptions::ttdDisableAutoTrace); #endif - Insert(&PerIsolateOptionsParser::instance, + Insert(PerIsolateOptionsParser::GetInstance(), &PerProcessOptions::get_per_isolate_options); } -PerProcessOptionsParser PerProcessOptionsParser::instance; - inline std::string RemoveBrackets(const std::string& host) { if (!host.empty() && host.front() == '[' && host.back() == ']') return host.substr(1, host.size() - 2); @@ -417,7 +409,7 @@ void GetOptions(const FunctionCallbackInfo& args) { per_process_opts->per_isolate = original_per_isolate; }); - const auto& parser = PerProcessOptionsParser::instance; + const auto& parser = *(PerProcessOptionsParser::GetInstance()); std::string filter; if (args[0]->IsString()) filter = *node::Utf8Value(isolate, args[0]); diff --git a/src/node_options.h b/src/node_options.h index bb26fe6402a..8fff1fcef34 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -278,6 +278,19 @@ class OptionsParser { OptionEnvvarSettings required_env_settings, std::vector* const errors); + + protected: + bool isInit = false; + + void Ensure() { + if (!isInit) { + Initialize(); + isInit = true; + } + } + + virtual void Initialize() {} + private: // We support the wide variety of different option types by remembering // how to access them, given a certain `Options` struct. @@ -363,32 +376,43 @@ class OptionsParser { friend void GetOptions(const v8::FunctionCallbackInfo& args); }; +#define OptionsParserGetInstance(type) \ + static type* GetInstance() { \ + static type instance; \ + instance.Ensure(); \ + return &instance; \ + } + class DebugOptionsParser : public OptionsParser { public: - DebugOptionsParser(); + OptionsParserGetInstance(DebugOptionsParser); - static DebugOptionsParser instance; + protected: + void Initialize(); }; class EnvironmentOptionsParser : public OptionsParser { public: - EnvironmentOptionsParser(); + OptionsParserGetInstance(EnvironmentOptionsParser); - static EnvironmentOptionsParser instance; + protected: + void Initialize(); }; class PerIsolateOptionsParser : public OptionsParser { public: - PerIsolateOptionsParser(); + OptionsParserGetInstance(PerIsolateOptionsParser); - static PerIsolateOptionsParser instance; + protected: + void Initialize(); }; class PerProcessOptionsParser : public OptionsParser { public: - PerProcessOptionsParser(); + OptionsParserGetInstance(PerProcessOptionsParser); - static PerProcessOptionsParser instance; + protected: + void Initialize(); }; } // namespace options_parser