core: envoy engine (#498)

Description: create a class to manage Envoy lifecycle and allow for graceful teardown. Teardown is now graceful for both iOS and Android. Envoy's codebase expects to have shutdown happen from the main thread. However, in mobile the thread the engine runs in is not the main thread of the process, and thus shutdown/destructors were not getting run from the thread the engine was started on. This PR makes sure that the engine is destructed/shutdown from the thread that it ran in.
Risk Level: high. This PR changes state management for the engine, and initializes it on a std::thread instead of a platform thread. iOS thread management is afaict handled gracefully. On Android the native thread has to be attached and detached from the JVM. This PR attaches the native thread, but the work to detach is a bit involved so will come in a subsequent PR.
Testing: local device testing.

Fixes #492

Co-authored-by: Jose Nino <[email protected]>
Signed-off-by: Mike Schore <[email protected]>
Signed-off-by: JP Simard <[email protected]>

mobile/library/common/BUILD

@@ -9,6 +9,8 @@ envoy_cc_library(
     srcs = [
         "certificates.inc",
         "config_template.cc",
+        "engine.cc",
+        "engine.h",
         "main_interface.cc",
     ],
     hdrs = ["main_interface.h"],

mobile/library/common/engine.cc

@@ -0,0 +1,101 @@
+#include "library/common/engine.h"
+
+#include "common/common/lock_guard.h"
+
+namespace Envoy {
+
+// As a server, Envoy's static factory registration happens when main is run. However, when compiled
+// as a library, there is no guarantee that such registration will happen before the names are
+// needed. The following calls ensure that registration happens before the entities are needed. Note
+// that as more registrations are needed, explicit initialization calls will need to be added here.
+static void registerFactories() {
+  Envoy::Extensions::Clusters::DynamicForwardProxy::forceRegisterClusterFactory();
+  Envoy::Extensions::HttpFilters::DynamicForwardProxy::
+      forceRegisterDynamicForwardProxyFilterFactory();
+  Envoy::Extensions::HttpFilters::RouterFilter::forceRegisterRouterFilterConfig();
+  Envoy::Extensions::NetworkFilters::HttpConnectionManager::
+      forceRegisterHttpConnectionManagerFilterConfigFactory();
+  Envoy::Extensions::TransportSockets::RawBuffer::forceRegisterDownstreamRawBufferSocketFactory();
+  Envoy::Extensions::TransportSockets::RawBuffer::forceRegisterUpstreamRawBufferSocketFactory();
+  Envoy::Extensions::TransportSockets::Tls::forceRegisterUpstreamSslSocketFactory();
+  Envoy::Upstream::forceRegisterLogicalDnsClusterFactory();
+}
+
+Engine::Engine(const char* config, const char* log_level,
+               std::atomic<envoy_network_t>& preferred_network) {
+  // Ensure static factory registration occurs on time.
+  // TODO: ensure this is only called one time once multiple Engine objects can be allocated.
+  registerFactories();
+
+  // Create the Http::Dispatcher first since it contains initial queueing logic.
+  // TODO: consider centralizing initial queueing in this class.
+  http_dispatcher_ = std::make_unique<Http::Dispatcher>(preferred_network);
+
+  // Start the Envoy on a dedicated thread.
+  main_thread_ = std::thread(&Engine::run, this, std::string(config), std::string(log_level));
+}
+
+envoy_status_t Engine::run(std::string config, std::string log_level) {
+  {
+    Thread::LockGuard lock(mutex_);
+    try {
+      char* envoy_argv[] = {strdup("envoy"), strdup("--config-yaml"),   strdup(config.c_str()),
+                            strdup("-l"),    strdup(log_level.c_str()), nullptr};
+
+      main_common_ = std::make_unique<Envoy::MainCommon>(5, envoy_argv);
+      event_dispatcher_ = &main_common_->server()->dispatcher();
+      cv_.notifyOne();
+    } catch (const Envoy::NoServingException& e) {
+      return ENVOY_FAILURE;
+    } catch (const Envoy::MalformedArgvException& e) {
+      std::cerr << e.what() << std::endl;
+      return ENVOY_FAILURE;
+    } catch (const Envoy::EnvoyException& e) {
+      std::cerr << e.what() << std::endl;
+      return ENVOY_FAILURE;
+    }
+
+    // Note: We're waiting longer than we might otherwise to drain to the main thread's dispatcher.
+    // This is because we're not simply waiting for its availability and for it to have started, but
+    // also because we're waiting for clusters to have done their first attempt at DNS resolution.
+    // When we improve synchronous failure handling and/or move to dynamic forwarding, we only need
+    // to wait until the dispatcher is running (and can drain by enqueueing a drain callback on it,
+    // as we did previously).
+    auto server = main_common_->server();
+    postinit_callback_handler_ = main_common_->server()->lifecycleNotifier().registerCallback(
+        Envoy::Server::ServerLifecycleNotifier::Stage::PostInit, [this, server]() -> void {
+          http_dispatcher_->ready(server->dispatcher(), server->clusterManager());
+        });
+  } // mutex_
+
+  // The main run loop must run without holding the mutex, so that the destructor can acquire it.
+  return TS_UNCHECKED_READ(main_common_)->run() ? ENVOY_SUCCESS : ENVOY_FAILURE;
+}
+
+Engine::~Engine() {
+  // If we're already on the main thread, it should be safe to simply destruct.
+  if (!main_thread_.joinable()) {
+    return;
+  }
+
+  // If we're not on the main thread, we need to be sure that MainCommon is finished being
+  // constructed so we can dispatch shutdown.
+  {
+    Thread::LockGuard lock(mutex_);
+    if (!main_common_) {
+      cv_.wait(mutex_);
+    }
+    ASSERT(main_common_);
+    // Gracefully shutdown the running envoy instance by resetting the main_common_ unique_ptr.
+    // Destroying MainCommon's member variables shutsdown things in the correct order and
+    // gracefully.
+    event_dispatcher_->post([this]() -> void { TS_UNCHECKED_READ(main_common_).reset(); });
+  } // _mutex
+
+  // Now we wait for the main thread to wrap things up.
+  main_thread_.join();
+}
+
+Http::Dispatcher& Engine::httpDispatcher() { return *http_dispatcher_; }
+
+} // namespace Envoy

mobile/library/common/engine.h

@@ -0,0 +1,43 @@
+#pragma once
+
+#include "envoy/server/lifecycle_notifier.h"
+
+#include "common/upstream/logical_dns_cluster.h"
+
+#include "exe/main_common.h"
+
+#include "extensions/clusters/dynamic_forward_proxy/cluster.h"
+#include "extensions/filters/http/dynamic_forward_proxy/config.h"
+#include "extensions/filters/http/router/config.h"
+#include "extensions/filters/network/http_connection_manager/config.h"
+#include "extensions/transport_sockets/raw_buffer/config.h"
+#include "extensions/transport_sockets/tls/config.h"
+
+#include "absl/base/call_once.h"
+#include "library/common/http/dispatcher.h"
+#include "library/common/types/c_types.h"
+
+namespace Envoy {
+
+class Engine {
+public:
+  Engine(const char* config, const char* log_level,
+         std::atomic<envoy_network_t>& preferred_network);
+
+  ~Engine();
+
+  envoy_status_t run(std::string config, std::string log_level);
+
+  Http::Dispatcher& httpDispatcher();
+
+private:
+  Thread::MutexBasicLockable mutex_;
+  Thread::CondVar cv_;
+  std::thread main_thread_;
+  std::unique_ptr<Envoy::Http::Dispatcher> http_dispatcher_;
+  std::unique_ptr<Envoy::MainCommon> main_common_ GUARDED_BY(mutex_);
+  Envoy::Server::ServerLifecycleNotifier::HandlePtr postinit_callback_handler_;
+  Event::Dispatcher* event_dispatcher_;
+};
+
+} // namespace Envoy

mobile/library/common/jni_interface.cc

@@ -30,11 +30,9 @@ extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibr
   return init_engine();
 }
 
-extern "C" JNIEXPORT jint JNICALL
-Java_io_envoyproxy_envoymobile_engine_JniLibrary_runEngine(JNIEnv* env,
-                                                           jclass, // class
-                                                           jstring config, jstring log_level) {
-  return run_engine(env->GetStringUTFChars(config, nullptr),
+extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_runEngine(
+    JNIEnv* env, jclass, jlong engine, jstring config, jstring log_level) {
+  return run_engine(engine, env->GetStringUTFChars(config, nullptr),
                     env->GetStringUTFChars(log_level, nullptr));
 }
 
@@ -121,6 +119,11 @@ static JNIEnv* get_env() {
   int get_env_res = static_jvm->GetEnv((void**)&env, JNI_VERSION);
   if (get_env_res == JNI_EDETACHED) {
     __android_log_write(ANDROID_LOG_ERROR, "jni_lib", "equals JNI_EDETACHED");
+    // Note: the only thread that should need to be attached is Envoy's engine std::thread.
+    // TODO: harden this piece of code to make sure that we are only needing to attach Envoy
+    // engine's std::thread, and that we detach it successfully.
+    static_jvm->AttachCurrentThread(&env, NULL);
+    static_jvm->GetEnv((void**)&env, JNI_VERSION);
   }
   return env;
 }

mobile/library/common/main_interface.cc

@@ -1,59 +1,43 @@
 #include "library/common/main_interface.h"
 
 #include <atomic>
-#include <unordered_map>
 
-#include "envoy/server/lifecycle_notifier.h"
-
-#include "common/upstream/logical_dns_cluster.h"
-
-#include "exe/main_common.h"
-
-#include "extensions/clusters/dynamic_forward_proxy/cluster.h"
-#include "extensions/filters/http/dynamic_forward_proxy/config.h"
-#include "extensions/filters/http/router/config.h"
-#include "extensions/filters/network/http_connection_manager/config.h"
-#include "extensions/transport_sockets/raw_buffer/config.h"
-#include "extensions/transport_sockets/tls/config.h"
-
-#include "library/common/buffer/utility.h"
+#include "library/common/engine.h"
 #include "library/common/http/dispatcher.h"
-#include "library/common/http/header_utility.h"
 
 // NOLINT(namespace-envoy)
 
-static std::unique_ptr<Envoy::MainCommon> main_common_;
-static std::unique_ptr<Envoy::Http::Dispatcher> http_dispatcher_;
-static Envoy::Server::ServerLifecycleNotifier::HandlePtr stageone_callback_handler_;
+static std::unique_ptr<Envoy::Engine> engine_;
 static std::atomic<envoy_stream_t> current_stream_handle_{0};
 static std::atomic<envoy_network_t> preferred_network_{ENVOY_NET_GENERIC};
 
 envoy_stream_t init_stream(envoy_engine_t) { return current_stream_handle_++; }
 
 envoy_status_t start_stream(envoy_stream_t stream, envoy_http_callbacks callbacks) {
-  http_dispatcher_->startStream(stream, callbacks);
+  engine_->httpDispatcher().startStream(stream, callbacks);
   return ENVOY_SUCCESS;
 }
 
 envoy_status_t send_headers(envoy_stream_t stream, envoy_headers headers, bool end_stream) {
-  return http_dispatcher_->sendHeaders(stream, headers, end_stream);
+  return engine_->httpDispatcher().sendHeaders(stream, headers, end_stream);
 }
 
 envoy_status_t send_data(envoy_stream_t stream, envoy_data data, bool end_stream) {
-  return http_dispatcher_->sendData(stream, data, end_stream);
+  return engine_->httpDispatcher().sendData(stream, data, end_stream);
 }
 
 // TODO: implement.
 envoy_status_t send_metadata(envoy_stream_t, envoy_headers) { return ENVOY_FAILURE; }
 
 envoy_status_t send_trailers(envoy_stream_t stream, envoy_headers trailers) {
-  return http_dispatcher_->sendTrailers(stream, trailers);
+  return engine_->httpDispatcher().sendTrailers(stream, trailers);
 }
 
-envoy_status_t reset_stream(envoy_stream_t stream) { return http_dispatcher_->resetStream(stream); }
+envoy_status_t reset_stream(envoy_stream_t stream) {
+  return engine_->httpDispatcher().resetStream(stream);
+}
 
 envoy_engine_t init_engine() {
-  http_dispatcher_ = std::make_unique<Envoy::Http::Dispatcher>(preferred_network_);
   // TODO(goaway): return new handle once multiple engine support is in place.
   // https://github.com/lyft/envoy-mobile/issues/332
   return 1;
@@ -67,62 +51,9 @@ envoy_status_t set_preferred_network(envoy_network_t network) {
 /**
  * External entrypoint for library.
  */
-envoy_status_t run_engine(const char* config, const char* log_level) {
-  char* envoy_argv[] = {strdup("envoy"), strdup("--config-yaml"), strdup(config),
-                        strdup("-l"),    strdup(log_level),       nullptr};
-
-  // Ensure static factory registration occurs on time.
-  // Envoy's static factory registration happens when main is run.
-  // However, when compiled as a library, there is no guarantee that such registration will happen
-  // before the names are needed.
-  // The following calls ensure that registration happens before the entities are needed.
-  // Note that as more registrations are needed, explicit initialization calls will need to be added
-  // here.
-  Envoy::Extensions::Clusters::DynamicForwardProxy::forceRegisterClusterFactory();
-  Envoy::Extensions::HttpFilters::DynamicForwardProxy::
-      forceRegisterDynamicForwardProxyFilterFactory();
-  Envoy::Extensions::HttpFilters::RouterFilter::forceRegisterRouterFilterConfig();
-  Envoy::Extensions::NetworkFilters::HttpConnectionManager::
-      forceRegisterHttpConnectionManagerFilterConfigFactory();
-  Envoy::Extensions::TransportSockets::RawBuffer::forceRegisterDownstreamRawBufferSocketFactory();
-  Envoy::Extensions::TransportSockets::RawBuffer::forceRegisterUpstreamRawBufferSocketFactory();
-  Envoy::Extensions::TransportSockets::Tls::forceRegisterUpstreamSslSocketFactory();
-  Envoy::Upstream::forceRegisterLogicalDnsClusterFactory();
-
-  // 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.
-  // Note that in the Android examples logging will not be seen.
-  // This is a known problem, and will be addressed by:
-  // https://github.com/lyft/envoy-mobile/issues/34
-  try {
-    main_common_ = std::make_unique<Envoy::MainCommon>(5, envoy_argv);
-
-    // Note: init_engine must have been called prior to calling run_engine or the creation of the
-    // envoy runner thread.
-
-    // Note: We're waiting longer than we might otherwise to drain to the main thread's dispatcher.
-    // This is because we're not simply waiting for its availability and for it to have started, but
-    // also because we're waiting for clusters to have done their first attempt at DNS resolution.
-    // When we improve synchronous failure handling and/or move to dynamic forwarding, we only need
-    // to wait until the dispatcher is running (and can drain by enqueueing a drain callback on it,
-    // as we did previously).
-    stageone_callback_handler_ = main_common_->server()->lifecycleNotifier().registerCallback(
-        Envoy::Server::ServerLifecycleNotifier::Stage::PostInit, []() -> void {
-          http_dispatcher_->ready(main_common_->server()->dispatcher(),
-                                  main_common_->server()->clusterManager());
-        });
-  } catch (const Envoy::NoServingException& e) {
-    return ENVOY_SUCCESS;
-  } catch (const Envoy::MalformedArgvException& e) {
-    std::cerr << e.what() << std::endl;
-    return ENVOY_FAILURE;
-  } catch (const Envoy::EnvoyException& e) {
-    std::cerr << e.what() << std::endl;
-    return ENVOY_FAILURE;
-  }
-
-  // Run the server listener loop outside try/catch blocks, so that unexpected
-  // exceptions show up as a core-dumps for easier diagnostics.
-  return main_common_->run() ? ENVOY_SUCCESS : ENVOY_FAILURE;
+envoy_status_t run_engine(envoy_engine_t, const char* config, const char* log_level) {
+  // This will change once multiple engine support is in place.
+  // https://github.com/lyft/envoy-mobile/issues/332
+  engine_ = std::make_unique<Envoy::Engine>(config, log_level, preferred_network_);
+  return ENVOY_SUCCESS;
 }

mobile/library/common/main_interface.h

@@ -93,11 +93,12 @@ envoy_status_t set_preferred_network(envoy_network_t network);
 
 /**
  * External entry point for library.
+ * @param engine, handle to the engine to run.
  * @param config, the configuration blob to run envoy with.
  * @param log_level, the logging level to run envoy with.
  * @return envoy_status_t, the resulting status of the operation.
  */
-envoy_status_t run_engine(const char* config, const char* log_level);
+envoy_status_t run_engine(envoy_engine_t engine, const char* config, const char* log_level);
 
 #ifdef __cplusplus
 } // functions

mobile/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java

@@ -32,7 +32,7 @@ public EnvoyHTTPStream startStream(EnvoyHTTPCallbacks callbacks) {
   @Override
   public int runWithConfig(String configurationYAML, String logLevel) {
     try {
-      return JniLibrary.runEngine(configurationYAML, logLevel);
+      return JniLibrary.runEngine(this.engineHandle, configurationYAML, logLevel);
     } catch (Throwable throwable) {
       // TODO: Need to have a way to log the exception somewhere
       return 1;
@@ -43,7 +43,7 @@ public int runWithConfig(String configurationYAML, String logLevel) {
    * Run the Envoy engine with the provided envoyConfiguration and log level.
    *
    * @param envoyConfiguration The EnvoyConfiguration used to start Envoy.
-   * @param logLevel The log level to use when starting Envoy.
+   * @param logLevel           The log level to use when starting Envoy.
    * @return int A status indicating if the action was successful.
    */
   @Override

mobile/library/java/src/io/envoyproxy/envoymobile/engine/JniLibrary.java

@@ -131,11 +131,12 @@ private static class JavaLoader {
   /**
    * External entry point for library.
    *
+   * @param engine,   the engine to run.
    * @param config,   the configuration blob to run envoy with.
    * @param logLevel, the logging level to run envoy with.
    * @return int, the resulting status of the operation.
    */
-  protected static native int runEngine(String config, String logLevel);
+  protected static native int runEngine(long engine, String config, String logLevel);
 
   // Other native methods
 

mobile/library/objective-c/EnvoyEngineImpl.m

@@ -32,7 +32,7 @@ - (int)runWithConfigYAML:(NSString *)configYAML logLevel:(NSString *)logLevel {
   // Envoy exceptions will only be caught here when compiled for 64-bit arches.
   // https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Exceptions/Articles/Exceptions64Bit.html
   @try {
-    return (int)run_engine(configYAML.UTF8String, logLevel.UTF8String);
+    return (int)run_engine(_engineHandle, configYAML.UTF8String, logLevel.UTF8String);
   } @catch (...) {
     NSLog(@"Envoy exception caught.");
     [NSNotificationCenter.defaultCenter postNotificationName:@"EnvoyException" object:self];

mobile/test/performance/test_binary_size.cc

@@ -5,4 +5,4 @@
 // This binary is used to perform stripped down binary size investigations of the Envoy codebase.
 // Please refer to the development docs for more information:
 // https://envoy-mobile.github.io/docs/envoy-mobile/latest/development/performance/binary_size.html
-int main() { return run_engine(nullptr, nullptr); }
+int main() { return run_engine(0, nullptr, nullptr); }