Fix logdeviced crash if ServerParams initialization fails. Fixes #133 (…

…#134) Summary: Pull Request resolved: #134 If an exception happens during the initialization of ServerParmeters, we do an ld_critical which invokes bumpErrorCounter that uses the StatsHolder of the server, but the Server is not there anymore so we crash. This case is mostly handled in the destructor of ServerParameters but it doesn't get called because of the exception. This diff does the init logic in an init() function instead of the constructor. Reviewed By: tau0 Differential Revision: D18138159 fbshipit-source-id: a968e31714717b96dcdf6dc4797961d30180eac8
facebookarchive · Oct 28, 2019 · 4f3366d · 4f3366d
1 parent b1bf9eb
commit 4f3366d
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 12 deletions.
diff --git a/logdevice/server/Server.cpp b/logdevice/server/Server.cpp
@@ -357,7 +357,9 @@ ServerParameters::ServerParameters(
       admin_server_settings_(std::move(admin_server_settings)),
       stop_handler_(std::move(stop_handler)) {
   ld_check(stop_handler_);
+}
 
+void ServerParameters::init() {
   // Note: this won't work well if there are multiple Server instances in the
   // same process: only one of them will get its error counter bumped. Also,
   // there's a data race if the following two lines are run from multiple

diff --git a/logdevice/server/Server.h b/logdevice/server/Server.h
@@ -78,6 +78,9 @@ class ServerParameters {
   ServerParameters(const ServerParameters& rhs) = delete;
   ServerParameters& operator=(const ServerParameters& rhs) = delete;
 
+  // Must be called before passing the params object to the server.
+  void init();
+
   // Not mutually exclusive.
   bool isStorageNode() const;
   bool isSequencingEnabled() const;

diff --git a/logdevice/server/main.cpp b/logdevice/server/main.cpp
@@ -424,20 +424,21 @@ int main(int argc, const char** argv) {
     ld_info("asserts on (NDEBUG not set)");
   }
 
-  std::unique_ptr<ServerParameters> params;
+  auto params = std::make_unique<ServerParameters>(settings_updater,
+                                                   server_settings,
+                                                   rebuilding_settings,
+                                                   locallogstore_settings,
+                                                   gossip_settings,
+                                                   settings,
+                                                   rocksdb_settings,
+                                                   admin_server_settings,
+                                                   plugin_registry,
+                                                   signal_shutdown);
   try {
-    params = std::make_unique<ServerParameters>(settings_updater,
-                                                server_settings,
-                                                rebuilding_settings,
-                                                locallogstore_settings,
-                                                gossip_settings,
-                                                settings,
-                                                rocksdb_settings,
-                                                admin_server_settings,
-                                                plugin_registry,
-                                                signal_shutdown);
+    params->init();
   } catch (const ConstructorFailed&) {
-    ld_critical("Unable to instantiate ServerParameters. Exiting.");
+    params.reset();
+    ld_critical("Unable to initialize ServerParameters. Exiting.");
     return 1;
   }