shutdown: create avenues for shutdown out of static de-initialization (…

…#1021)

Description: this PR attempts to fix two related crashes. By hooking up to iOS's notification system we attempt to terminate the engine earlier, and allow all objects necessary to be available fixing #1035. Potentially fixes #831.
Risk Level: med - changes termination sequence
Testing: added test

Signed-off-by: Jose Nino <jnino@lyft.com>

library/common/BUILD

@@ -6,6 +6,15 @@ envoy_package()
 
 envoy_cc_library(
     name = "envoy_main_interface_lib",
+    repository = "@envoy",
+    deps = [
+        ":envoy_main_interface_lib_no_stamp",
+        ":envoy_mobile_main_common_lib_stamped",
+    ],
+)
+
+envoy_cc_library(
+    name = "envoy_main_interface_lib_no_stamp",
     srcs = [
         "certificates.inc",
         "config_template.cc",
@@ -16,7 +25,7 @@ envoy_cc_library(
     hdrs = ["main_interface.h"],
     repository = "@envoy",
     deps = [
-        ":envoy_mobile_main_common_lib_stamped",
+        ":envoy_mobile_main_common_lib",
         "//library/common/buffer:utility_lib",
         "//library/common/http:dispatcher_lib",
         "//library/common/http:header_utility_lib",

library/common/engine.cc

@@ -21,17 +21,21 @@ Engine::Engine(envoy_engine_callbacks callbacks, const char* config, const char*
   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) {
+envoy_status_t Engine::run(const std::string config, const 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};
+      const std::string name = "envoy";
+      const std::string config_flag = "--config-yaml";
+      const std::string log_flag = "-l";
+      const char* envoy_argv[] = {name.c_str(),     config_flag.c_str(), config.c_str(),
+                                  log_flag.c_str(), log_level.c_str(),   nullptr};
 
       main_common_ = std::make_unique<MobileMainCommon>(5, envoy_argv);
       event_dispatcher_ = &main_common_->server()->dispatcher();
-      cv_.notifyOne();
+      cv_.notifyAll();
     } catch (const Envoy::NoServingException& e) {
+      std::cerr << e.what() << std::endl;
       return ENVOY_FAILURE;
     } catch (const Envoy::MalformedArgvException& e) {
       std::cerr << e.what() << std::endl;
@@ -60,15 +64,15 @@ envoy_status_t Engine::run(std::string config, std::string log_level) {
 
   // The main run loop must run without holding the mutex, so that the destructor can acquire it.
   bool run_success = TS_UNCHECKED_READ(main_common_)->run();
-
   // The above call is blocking; at this point the event loop has exited.
-  callbacks_.on_exit();
 
   // Ensure destructors run on Envoy's main thread.
   postinit_callback_handler_.reset(nullptr);
   client_scope_.reset(nullptr);
   TS_UNCHECKED_READ(main_common_).reset(nullptr);
 
+  callbacks_.on_exit(callbacks_.context);
+
   return run_success ? ENVOY_SUCCESS : ENVOY_FAILURE;
 }
 
@@ -82,9 +86,11 @@ Engine::~Engine() {
   // constructed so we can dispatch shutdown.
   {
     Thread::LockGuard lock(mutex_);
+
     if (!main_common_) {
       cv_.wait(mutex_);
     }
+
     ASSERT(main_common_);
 
     // Exit the event loop and finish up in Engine::run(...)

library/common/jni/jni_interface.cc

@@ -30,7 +30,7 @@ extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibr
   return init_engine();
 }
 
-static void jvm_on_exit() {
+static void jvm_on_exit(void*) {
   __android_log_write(ANDROID_LOG_INFO, "[Envoy]", "library is exiting");
   // Note that this is not dispatched because the thread that
   // needs to be detached is the engine thread.
@@ -41,7 +41,7 @@ static void jvm_on_exit() {
 
 extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_runEngine(
     JNIEnv* env, jclass, jlong engine, jstring config, jstring log_level) {
-  envoy_engine_callbacks native_callbacks = {jvm_on_exit};
+  envoy_engine_callbacks native_callbacks = {jvm_on_exit, nullptr};
   return run_engine(engine, native_callbacks, env->GetStringUTFChars(config, nullptr),
                     env->GetStringUTFChars(log_level, nullptr));
 }

library/common/main_interface.cc

@@ -10,6 +10,7 @@
 
 // NOLINT(namespace-envoy)
 
+static std::shared_ptr<Envoy::Engine> strong_engine_;
 static std::weak_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};
@@ -95,12 +96,14 @@ envoy_status_t run_engine(envoy_engine_t, envoy_engine_callbacks callbacks, cons
   // https://github.com/lyft/envoy-mobile/issues/332
 
   // The shared pointer created here will keep the engine alive until static destruction occurs.
-  static auto strong_ref =
+  strong_engine_ =
       std::make_shared<Envoy::Engine>(callbacks, config, log_level, preferred_network_);
 
   // The weak pointer we actually expose allows calling threads to atomically check if the engine
   // still exists and acquire a shared pointer to it - ensuring the engine persists at least for
   // the duration of the call.
-  engine_ = strong_ref;
+  engine_ = strong_engine_;
   return ENVOY_SUCCESS;
 }
+
+void terminate_engine(envoy_engine_t) { strong_engine_.reset(); }

library/common/main_interface.h

@@ -128,6 +128,8 @@ envoy_status_t register_platform_api(const char* name, void* api);
 envoy_status_t run_engine(envoy_engine_t engine, envoy_engine_callbacks callbacks,
                           const char* config, const char* log_level);
 
+void terminate_engine(envoy_engine_t engine);
+
 #ifdef __cplusplus
 } // functions
 #endif

library/common/types/c_types.h

@@ -231,7 +231,7 @@ typedef void* (*envoy_on_cancel_f)(void* context);
 /**
  * Called when the envoy engine is exiting.
  */
-typedef void (*envoy_on_exit_f)();
+typedef void (*envoy_on_exit_f)(void* context);
 
 #ifdef __cplusplus
 } // function pointers
@@ -260,4 +260,5 @@ typedef struct {
  */
 typedef struct {
   envoy_on_exit_f on_exit;
+  void* context;
 } envoy_engine_callbacks;

library/objective-c/EnvoyEngineImpl.m

@@ -7,7 +7,7 @@
 
 #import <UIKit/UIKit.h>
 
-static void ios_on_exit() {
+static void ios_on_exit(void *context) {
   // Currently nothing needs to happen in iOS on exit. Just log.
   NSLog(@"[Envoy] library is exiting");
 }
@@ -164,13 +164,12 @@ - (int)runWithConfig:(EnvoyConfiguration *)config logLevel:(NSString *)logLevel
 }
 
 - (int)runWithConfigYAML:(NSString *)configYAML logLevel:(NSString *)logLevel {
-  // re-enable lifecycle-based stat flushing when https://github.com/lyft/envoy-mobile/issues/748
-  // gets fixed. [self startObservingLifecycleNotifications];
+  [self startObservingLifecycleNotifications];
 
   // 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 {
-    envoy_engine_callbacks native_callbacks = {ios_on_exit};
+    envoy_engine_callbacks native_callbacks = {ios_on_exit, NULL};
     return (int)run_engine(_engineHandle, native_callbacks, configYAML.UTF8String,
                            logLevel.UTF8String);
   } @catch (NSException *exception) {
@@ -192,20 +191,18 @@ - (void)recordCounter:(NSString *)elements count:(NSUInteger)count {
 #pragma mark - Private
 
 - (void)startObservingLifecycleNotifications {
+  // re-enable lifecycle-based stat flushing when https://github.com/lyft/envoy-mobile/issues/748
+  // gets fixed.
   NSNotificationCenter *notificationCenter = [NSNotificationCenter defaultCenter];
   [notificationCenter addObserver:self
-                         selector:@selector(lifecycleDidChangeWithNotification:)
-                             name:UIApplicationWillResignActiveNotification
-                           object:nil];
-  [notificationCenter addObserver:self
-                         selector:@selector(lifecycleDidChangeWithNotification:)
+                         selector:@selector(terminateNotification:)
                              name:UIApplicationWillTerminateNotification
                            object:nil];
 }
 
-- (void)lifecycleDidChangeWithNotification:(NSNotification *)notification {
-  NSLog(@"[Envoy] triggering stats flush (%@)", notification.name);
-  flush_stats();
+- (void)terminateNotification:(NSNotification *)notification {
+  NSLog(@"[Envoy %ld] terminating engine (%@)", _engineHandle, notification.name);
+  terminate_engine(_engineHandle);
 }
 
 @end

test/common/BUILD

@@ -12,3 +12,13 @@ envoy_cc_test(
         "//library/common:envoy_mobile_main_common_lib",
     ],
 )
+
+envoy_cc_test(
+    name = "engine_test",
+    srcs = ["engine_test.cc"],
+    repository = "@envoy",
+    deps = [
+        "//library/common:envoy_main_interface_lib_no_stamp",
+        "//library/common/types:c_types_lib",
+    ],
+)

test/common/engine_test.cc

@@ -0,0 +1,41 @@
+#include "absl/synchronization/notification.h"
+#include "gtest/gtest.h"
+#include "library/common/engine.h"
+#include "library/common/main_interface.h"
+
+namespace Envoy {
+
+class EngineTest : public testing::Test {};
+
+TEST_F(EngineTest, EarlyExit) {
+  const std::string config =
+      "{\"admin\":{},\"static_resources\":{\"listeners\":[{\"name\":\"base_api_listener\","
+      "\"address\":{\"socket_address\":{\"protocol\":\"TCP\",\"address\":\"0.0.0.0\",\"port_"
+      "value\":10000}},\"api_listener\":{\"api_listener\":{\"@type\":\"type.googleapis.com/"
+      "envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager\",\"stat_"
+      "prefix\":\"hcm\",\"route_config\":{\"name\":\"api_router\",\"virtual_hosts\":[{\"name\":"
+      "\"api\",\"include_attempt_count_in_response\":true,\"domains\":[\"*\"],\"routes\":[{"
+      "\"match\":{\"prefix\":\"/"
+      "\"},\"route\":{\"cluster_header\":\"x-envoy-mobile-cluster\",\"retry_policy\":{\"retry_back_"
+      "off\":{\"base_interval\":\"0.25s\",\"max_interval\":\"60s\"}}}}]}]},\"http_filters\":[{"
+      "\"name\":\"envoy.router\",\"typed_config\":{\"@type\":\"type.googleapis.com/"
+      "envoy.extensions.filters.http.router.v3.Router\"}}]}}}]},\"layered_runtime\":{\"layers\":[{"
+      "\"name\":\"static_layer_0\",\"static_layer\":{\"overload\":{\"global_downstream_max_"
+      "connections\":50000}}}]}}";
+  const std::string level = "debug";
+  absl::Notification done;
+  envoy_engine_callbacks cbs{[](void* context) -> void {
+                               auto* done = static_cast<absl::Notification*>(context);
+                               done->Notify();
+                             },
+                             &done};
+
+  run_engine(0, cbs, config.c_str(), level.c_str());
+
+  terminate_engine(0);
+
+  ASSERT_TRUE(done.WaitForNotificationWithTimeout(absl::Seconds(1)));
+
+  start_stream(0, {});
+}
+} // namespace Envoy