Revert "server: factor out MainCommon as a class, with a run method (…

include/envoy/server/BUILD

@@ -57,10 +57,7 @@ envoy_cc_library(
 envoy_cc_library(
     name = "hot_restart_interface",
     hdrs = ["hot_restart.h"],
-    deps = [
-        "//include/envoy/event:dispatcher_interface",
-        "//include/envoy/thread:thread_interface",
-    ],
+    deps = ["//include/envoy/event:dispatcher_interface"],
 )
 
 envoy_cc_library(

include/envoy/server/hot_restart.h

@@ -5,8 +5,6 @@
 
 #include "envoy/common/pure.h"
 #include "envoy/event/dispatcher.h"
-#include "envoy/stats/stats.h"
-#include "envoy/thread/thread.h"
 
 namespace Envoy {
 namespace Server {
@@ -80,21 +78,6 @@ class HotRestart {
    * perform a full or hot restart.
    */
   virtual std::string version() PURE;
-
-  /**
-   * @return Thread::BasicLockable& a lock for logging.
-   */
-  virtual Thread::BasicLockable& logLock() PURE;
-
-  /**
-   * @return Thread::BasicLockable& a lock for access logs.
-   */
-  virtual Thread::BasicLockable& accessLogLock() PURE;
-
-  /**
-   * @returns an allocator for stats.
-   */
-  virtual Stats::RawStatDataAllocator& statsAllocator() PURE;
 };
 
 } // namespace Server

include/envoy/stats/stats.h

@@ -281,29 +281,5 @@ class StoreRoot : public Store {
 
 typedef std::unique_ptr<StoreRoot> StoreRootPtr;
 
-struct RawStatData;
-
-/**
- * Abstract interface for allocating a RawStatData.
- */
-class RawStatDataAllocator {
-public:
-  virtual ~RawStatDataAllocator() {}
-
-  /**
-   * @return RawStatData* a raw stat data block for a given stat name or nullptr if there is no
-   *         more memory available for stats. The allocator should return a reference counted
-   *         data location by name if one already exists with the same name. This is used for
-   *         intra-process scope swapping as well as inter-process hot restart.
-   */
-  virtual RawStatData* alloc(const std::string& name) PURE;
-
-  /**
-   * Free a raw stat data block. The allocator should handle reference counting and only truly
-   * free the block if it is no longer needed.
-   */
-  virtual void free(RawStatData& data) PURE;
-};
-
 } // namespace Stats
 } // namespace Envoy

source/common/stats/stats_impl.h

@@ -191,6 +191,28 @@ struct RawStatData {
   static size_t& initializeAndGetMutableMaxObjNameLength(size_t configured_size);
 };
 
+/**
+ * Abstract interface for allocating a RawStatData.
+ */
+class RawStatDataAllocator {
+public:
+  virtual ~RawStatDataAllocator() {}
+
+  /**
+   * @return RawStatData* a raw stat data block for a given stat name or nullptr if there is no more
+   *         memory available for stats. The allocator may return a reference counted data location
+   *         by name if one already exists with the same name. This is used for intra-process
+   *         scope swapping as well as inter-process hot restart.
+   */
+  virtual RawStatData* alloc(const std::string& name) PURE;
+
+  /**
+   * Free a raw stat data block. The allocator should handle reference counting and only truly
+   * free the block if it is no longer needed.
+   */
+  virtual void free(RawStatData& data) PURE;
+};
+
 /**
  * Implementation of the Metric interface. Virtual inheritance is used because the interfaces that
  * will inherit from Metric will have other base classes that will also inherit from Metric.

source/exe/BUILD

@@ -79,9 +79,14 @@ envoy_cc_library(
     deps = [
         ":envoy_main_common_lib",
         ":extra_protocol_proxies_lib",
+        "//source/server:hot_restart_lib",
+        "//source/server:options_lib",
         "//source/server/config/http:lightstep_lib",
         "//source/server/config/http:zipkin_lib",
-    ],
+    ] + select({
+        "//bazel:disable_signal_trace": [],
+        "//conditions:default": [":sigaction_lib"],
+    }),
 )
 
 envoy_cc_library(
@@ -96,10 +101,7 @@ envoy_cc_library(
         "//source/server:hot_restart_nop_lib",
         "//source/server:proto_descriptors_lib",
         "//source/server/config_validation:server_lib",
-    ] + select({
-        "//bazel:disable_signal_trace": [],
-        "//conditions:default": [":sigaction_lib"],
-    }),
+    ],
 )
 
 envoy_cc_library(

source/exe/main.cc

@@ -1,5 +1,20 @@
+#include <iostream>
+#include <memory>
+
 #include "exe/main_common.h"
 
+#ifdef ENVOY_HANDLE_SIGNALS
+#include "exe/signal_action.h"
+#endif
+
+#ifdef ENVOY_HOT_RESTART
+#include "server/hot_restart_impl.h"
+#endif
+
+#include "server/options_impl.h"
+
+#include "spdlog/spdlog.h"
+
 // NOLINT(namespace-envoy)
 
 /**
@@ -10,28 +25,31 @@
  * after setting up command line options.
  */
 int main(int argc, char** argv) {
+#ifdef ENVOY_HANDLE_SIGNALS
+  // Enabled by default. Control with "bazel --define=signal_trace=disabled"
+  Envoy::SignalAction handle_sigs;
+#endif
+
 #ifdef ENVOY_HOT_RESTART
-  constexpr bool enable_hot_restart = true;
+  // Enabled by default, except on OS X. Control with "bazel --define=hot_restart=disabled"
+  const Envoy::OptionsImpl::HotRestartVersionCb hot_restart_version_cb =
+      [](uint64_t max_num_stats, uint64_t max_stat_name_len) {
+        return Envoy::Server::HotRestartImpl::hotRestartVersion(max_num_stats, max_stat_name_len);
+      };
 #else
-  constexpr bool enable_hot_restart = false;
+  const Envoy::OptionsImpl::HotRestartVersionCb hot_restart_version_cb = [](uint64_t, uint64_t) {
+    return "disabled";
+  };
 #endif
 
-  std::unique_ptr<Envoy::MainCommon> main_common;
-
-  // Initialize the server's main context under a try/catch loop and simply return EXIT_FAILURE
-  // as needed. Whatever code in the initialization path that fails is expected to log an error
-  // message so the user can diagnose.
+  std::unique_ptr<Envoy::OptionsImpl> options;
   try {
-    main_common = std::make_unique<Envoy::MainCommon>(argc, argv, enable_hot_restart);
+    options = std::make_unique<Envoy::OptionsImpl>(argc, argv, hot_restart_version_cb,
+                                                   spdlog::level::info);
   } catch (const Envoy::NoServingException& e) {
-    return EXIT_SUCCESS;
+    return 0;
   } catch (const Envoy::MalformedArgvException& e) {
-    return EXIT_FAILURE;
-  } catch (const Envoy::EnvoyException& e) {
-    return EXIT_FAILURE;
+    return 1;
   }
-
-  // Run the server listener loop outside try/catch blocks, so that unexpected exceptions
-  // show up as a core-dumps for easier diagnostis.
-  return main_common->run() ? EXIT_SUCCESS : EXIT_FAILURE;
+  return Envoy::main_common(*options);
 }

source/exe/main_common.cc

@@ -1,12 +1,11 @@
-#include "exe/main_common.h"
-
 #include <iostream>
 #include <memory>
 
 #include "common/common/compiler_requirements.h"
 #include "common/event/libevent.h"
 #include "common/network/utility.h"
 #include "common/stats/stats_impl.h"
+#include "common/stats/thread_local_store.h"
 
 #include "server/config_validation/server.h"
 #include "server/drain_manager_impl.h"
@@ -23,107 +22,79 @@
 #include "ares.h"
 
 namespace Envoy {
+namespace Server {
+
+class ProdComponentFactory : public ComponentFactory {
+public:
+  // Server::DrainManagerFactory
+  DrainManagerPtr createDrainManager(Instance& server) override {
+    return DrainManagerPtr{
+        // The global drain manager only triggers on listener modification, which effectively is
+        // hot restart at the global level. The per-listener drain managers decide whether to
+        // to include /healthcheck/fail status.
+        new DrainManagerImpl(server, envoy::api::v2::Listener_DrainType_MODIFY_ONLY)};
+  }
 
-Server::DrainManagerPtr ProdComponentFactory::createDrainManager(Server::Instance& server) {
-  // The global drain manager only triggers on listener modification, which effectively is
-  // hot restart at the global level. The per-listener drain managers decide whether to
-  // to include /healthcheck/fail status.
-  return std::make_unique<Server::DrainManagerImpl>(server,
-                                                    envoy::api::v2::Listener_DrainType_MODIFY_ONLY);
-}
+  Runtime::LoaderPtr createRuntime(Server::Instance& server,
+                                   Server::Configuration::Initial& config) override {
+    return Server::InstanceUtil::createRuntime(server, config);
+  }
+};
 
-Runtime::LoaderPtr ProdComponentFactory::createRuntime(Server::Instance& server,
-                                                       Server::Configuration::Initial& config) {
-  return Server::InstanceUtil::createRuntime(server, config);
-}
+} // namespace Server
 
-MainCommonBase::MainCommonBase(OptionsImpl& options, bool hot_restart) : options_(options) {
-  ares_library_init(ARES_LIB_INIT_ALL);
-  Event::Libevent::Global::initialize();
-  RELEASE_ASSERT(Envoy::Server::validateProtoDescriptors());
+int main_common(OptionsImpl& options) {
+  Stats::RawStatData::configure(options);
 
-  switch (options_.mode()) {
-  case Server::Mode::Serve: {
 #ifdef ENVOY_HOT_RESTART
-    if (hot_restart) {
-      restarter_.reset(new Server::HotRestartImpl(options_));
-    }
-#endif
-    if (!hot_restart) {
-      restarter_.reset(new Server::HotRestartNopImpl());
-    }
-
-    Stats::RawStatData::configure(options_);
-    tls_.reset(new ThreadLocal::InstanceImpl);
-    Thread::BasicLockable& log_lock = restarter_->logLock();
-    Thread::BasicLockable& access_log_lock = restarter_->accessLogLock();
-    auto local_address = Network::Utility::getLocalAddress(options_.localAddressIpVersion());
-    Logger::Registry::initialize(options_.logLevel(), log_lock);
-
-    stats_store_.reset(new Stats::ThreadLocalStoreImpl(restarter_->statsAllocator()));
-    server_.reset(new Server::InstanceImpl(options_, local_address, default_test_hooks_,
-                                           *restarter_, *stats_store_, access_log_lock,
-                                           component_factory_, *tls_));
-    break;
-  }
-  case Server::Mode::Validate:
-    break;
+  std::unique_ptr<Server::HotRestartImpl> restarter;
+  try {
+    restarter.reset(new Server::HotRestartImpl(options));
+  } catch (Envoy::EnvoyException& e) {
+    std::cerr << "unable to initialize hot restart: " << e.what() << std::endl;
+    return 1;
   }
-}
 
-MainCommonBase::~MainCommonBase() { ares_library_cleanup(); }
+  Thread::BasicLockable& log_lock = restarter->logLock();
+  Thread::BasicLockable& access_log_lock = restarter->accessLogLock();
+  Stats::RawStatDataAllocator& stats_allocator = *restarter;
+#else
+  std::unique_ptr<Server::HotRestartNopImpl> restarter;
+  restarter.reset(new Server::HotRestartNopImpl());
+
+  Thread::MutexBasicLockable log_lock, access_log_lock;
+  Stats::HeapRawStatDataAllocator stats_allocator;
+#endif
 
-bool MainCommonBase::run() {
-  switch (options_.mode()) {
+  RELEASE_ASSERT(Envoy::Server::validateProtoDescriptors());
+  Event::Libevent::Global::initialize();
+  Server::ProdComponentFactory component_factory;
+  auto local_address = Network::Utility::getLocalAddress(options.localAddressIpVersion());
+  switch (options.mode()) {
   case Server::Mode::Serve:
-    server_->run();
-    return true;
-  case Server::Mode::Validate: {
-    auto local_address = Network::Utility::getLocalAddress(options_.localAddressIpVersion());
-    return Server::validateConfig(options_, local_address, component_factory_);
-  }
+    break;
+  case Server::Mode::Validate:
+    Thread::MutexBasicLockable log_lock;
+    Logger::Registry::initialize(options.logLevel(), log_lock);
+    return Server::validateConfig(options, local_address, component_factory) ? 0 : 1;
   }
-  NOT_REACHED;
-}
-
-MainCommon::MainCommon(int argc, char** argv, bool hot_restart)
-    : options_(computeOptions(argc, argv, hot_restart)), base_(*options_, hot_restart) {}
-
-std::unique_ptr<OptionsImpl> MainCommon::computeOptions(int argc, char** argv, bool hot_restart) {
-  OptionsImpl::HotRestartVersionCb hot_restart_version_cb = [](uint64_t, uint64_t) {
-    return "disabled";
-  };
 
-#ifdef ENVOY_HOT_RESTART
-  if (hot_restart) {
-    // Enabled by default, except on OS X. Control with "bazel --define=hot_restart=disabled"
-    hot_restart_version_cb = [](uint64_t max_num_stats, uint64_t max_stat_name_len) {
-      return Server::HotRestartImpl::hotRestartVersion(max_num_stats, max_stat_name_len);
-    };
-  }
-#else
-  // Hot-restart should not be specified if the support is not compiled in.
-  RELEASE_ASSERT(!hot_restart);
-#endif
-  return std::make_unique<OptionsImpl>(argc, argv, hot_restart_version_cb, spdlog::level::info);
-}
+  ares_library_init(ARES_LIB_INIT_ALL);
 
-// Legacy implementation of main_common.
-//
-// TODO(jmarantz): Remove this when all callers are removed. At that time, MainCommonBase
-// and MainCommon can be merged. The current theory is that only Google calls this.
-int main_common(OptionsImpl& options) {
+  Logger::Registry::initialize(options.logLevel(), log_lock);
+  DefaultTestHooks default_test_hooks;
+  ThreadLocal::InstanceImpl tls;
+  Stats::ThreadLocalStoreImpl stats_store(stats_allocator);
   try {
-#if ENVOY_HOT_RESTART
-    MainCommonBase main_common(options, true);
-#else
-    MainCommonBase main_common(options, false);
-#endif
-    return main_common.run() ? EXIT_SUCCESS : EXIT_FAILURE;
-  } catch (EnvoyException& e) {
-    return EXIT_FAILURE;
+    Server::InstanceImpl server(options, local_address, default_test_hooks, *restarter, stats_store,
+                                access_log_lock, component_factory, tls);
+    server.run();
+  } catch (const EnvoyException& e) {
+    ares_library_cleanup();
+    return 1;
   }
-  return EXIT_SUCCESS;
+  ares_library_cleanup();
+  return 0;
 }
 
 } // namespace Envoy