build: fix static initialization ordering problem in OptionsParser code

nodejs · Nov 15, 2018 · f02c844 · f02c844
1 parent 2169ac4
commit f02c844
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 25 deletions.
diff --git a/src/node.cc b/src/node.cc
@@ -2513,7 +2513,7 @@ void ProcessArgv(std::vector<std::string>* 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
@@ -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<Value>& 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
@@ -278,6 +278,19 @@ class OptionsParser {
                      OptionEnvvarSettings required_env_settings,
                      std::vector<std::string>* 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<v8::Value>& args);
 };
 
+#define OptionsParserGetInstance(type)                                         \
+  static type* GetInstance() {                                                 \
+    static type instance;                                                      \
+    instance.Ensure();                                                         \
+    return &instance;                                                          \
+  }
+
 class DebugOptionsParser : public OptionsParser<DebugOptions> {
  public:
-  DebugOptionsParser();
+  OptionsParserGetInstance(DebugOptionsParser);
 
-  static DebugOptionsParser instance;
+ protected:
+  void Initialize();
 };
 
 class EnvironmentOptionsParser : public OptionsParser<EnvironmentOptions> {
  public:
-  EnvironmentOptionsParser();
+  OptionsParserGetInstance(EnvironmentOptionsParser);
 
-  static EnvironmentOptionsParser instance;
+ protected:
+  void Initialize();
 };
 
 class PerIsolateOptionsParser : public OptionsParser<PerIsolateOptions> {
  public:
-  PerIsolateOptionsParser();
+  OptionsParserGetInstance(PerIsolateOptionsParser);
 
-  static PerIsolateOptionsParser instance;
+ protected:
+  void Initialize();
 };
 
 class PerProcessOptionsParser : public OptionsParser<PerProcessOptions> {
  public:
-  PerProcessOptionsParser();
+  OptionsParserGetInstance(PerProcessOptionsParser);
 
-  static PerProcessOptionsParser instance;
+ protected:
+  void Initialize();
 };
 
 }  // namespace options_parser