core: engine callbacks -- add on_exit (#507)

Description: this is a follow up to #498. This PR introduces `envoy_engine_callbacks`. They are similar in nature to envoy_http_callbacks. The difference being that they are not exposed all the way to consumer level in the library as it is not needed right now. However, one can see how by adding a type erased context pointer, and following the platform patterns for http callbacks we could thread this all the way up if need be. The immediate need for these callbacks is to detach the engine's native thread from the JVM on Android.
Risk Level: med -- adds complexity to engine management.
Testing: local testing on devices (Lyft and example app on iOS and Android).

In conjunction with #498 this PR Fixes #492 #445

Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: JP Simard <jp@jpsim.com>

mobile/library/common/engine.cc

@@ -21,8 +21,9 @@ static void registerFactories() {
   Envoy::Upstream::forceRegisterLogicalDnsClusterFactory();
 }
 
-Engine::Engine(const char* config, const char* log_level,
-               std::atomic<envoy_network_t>& preferred_network) {
+Engine::Engine(envoy_engine_callbacks callbacks, const char* config, const char* log_level,
+               std::atomic<envoy_network_t>& preferred_network)
+    : callbacks_(callbacks) {
   // Ensure static factory registration occurs on time.
   // TODO: ensure this is only called one time once multiple Engine objects can be allocated.
   registerFactories();
@@ -89,7 +90,10 @@ Engine::~Engine() {
     // 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(); });
+    event_dispatcher_->post([this]() -> void {
+      callbacks_.on_exit();
+      TS_UNCHECKED_READ(main_common_).reset();
+    });
   } // _mutex
 
   // Now we wait for the main thread to wrap things up.

mobile/library/common/engine.h

@@ -21,7 +21,7 @@ namespace Envoy {
 
 class Engine {
 public:
-  Engine(const char* config, const char* log_level,
+  Engine(envoy_engine_callbacks callbacks, const char* config, const char* log_level,
          std::atomic<envoy_network_t>& preferred_network);
 
   ~Engine();
@@ -31,6 +31,7 @@ class Engine {
   Http::Dispatcher& httpDispatcher();
 
 private:
+  envoy_engine_callbacks callbacks_;
   Thread::MutexBasicLockable mutex_;
   Thread::CondVar cv_;
   std::thread main_thread_;

mobile/library/common/jni_interface.cc

@@ -30,9 +30,19 @@ extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibr
   return init_engine();
 }
 
+static void jvm_on_exit() {
+  __android_log_write(ANDROID_LOG_ERROR, "jni_lib", "jvm_on_exit");
+  // Note that this is not dispatched because the thread that
+  // needs to be detached is the engine thread.
+  // This function is called from the context of the engine's
+  // thread due to it being posted to the engine's event dispatcher.
+  static_jvm->DetachCurrentThread();
+}
+
 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),
+  envoy_engine_callbacks native_callbacks = {jvm_on_exit};
+  return run_engine(engine, native_callbacks, env->GetStringUTFChars(config, nullptr),
                     env->GetStringUTFChars(log_level, nullptr));
 }
 
@@ -122,7 +132,7 @@ static JNIEnv* get_env() {
     // 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->AttachCurrentThread(&env, nullptr);
     static_jvm->GetEnv((void**)&env, JNI_VERSION);
   }
   return env;

mobile/library/common/main_interface.cc

@@ -51,9 +51,10 @@ envoy_status_t set_preferred_network(envoy_network_t network) {
 /**
  * External entrypoint for library.
  */
-envoy_status_t run_engine(envoy_engine_t, const char* config, const char* log_level) {
+envoy_status_t run_engine(envoy_engine_t, envoy_engine_callbacks callbacks, 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_);
+  engine_ = std::make_unique<Envoy::Engine>(callbacks, config, log_level, preferred_network_);
   return ENVOY_SUCCESS;
 }

mobile/library/common/main_interface.h

@@ -94,11 +94,13 @@ envoy_status_t set_preferred_network(envoy_network_t network);
 /**
  * External entry point for library.
  * @param engine, handle to the engine to run.
+ * @param callbacks, the callbacks that will run the engine callbacks.
  * @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(envoy_engine_t engine, const char* config, const char* log_level);
+envoy_status_t run_engine(envoy_engine_t engine, envoy_engine_callbacks callbacks,
+                          const char* config, const char* log_level);
 
 #ifdef __cplusplus
 } // functions

mobile/library/common/types/c_types.h

@@ -191,6 +191,11 @@ typedef void (*envoy_on_error_f)(envoy_error error, void* context);
  */
 typedef void (*envoy_on_complete_f)(void* context);
 
+/**
+ * Called when the envoy engine is exiting.
+ */
+typedef void (*envoy_on_exit_f)();
+
 #ifdef __cplusplus
 } // function pointers
 #endif
@@ -207,3 +212,14 @@ typedef struct {
   envoy_on_complete_f on_complete;
   void* context; // Will be passed through to callbacks to provide dispatch and execution state.
 } envoy_http_callbacks;
+
+/**
+ * Interface that can handle Engine callbacks.
+ * Note: currently this set of callbacks doesn't
+ * have a context because users of the library do not interact with the
+ * callbacks. However, these set of callbacks can be easily extended
+ * following the envoy_http_callbacks pattern to do so.
+ */
+typedef struct {
+  envoy_on_exit_f on_exit;
+} envoy_engine_callbacks;

mobile/library/objective-c/EnvoyEngineImpl.m

@@ -3,6 +3,11 @@
 #import "library/common/main_interface.h"
 #import "library/common/types/c_types.h"
 
+static void ios_on_exit() {
+  // Currently nothing needs to happen in iOS on exit. Just log.
+  NSLog(@"Envoy is exiting.");
+}
+
 @implementation EnvoyEngineImpl {
   envoy_engine_t _engineHandle;
 }
@@ -32,7 +37,9 @@ - (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(_engineHandle, configYAML.UTF8String, logLevel.UTF8String);
+    envoy_engine_callbacks native_callbacks = {ios_on_exit};
+    return (int)run_engine(_engineHandle, native_callbacks, 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(0, nullptr, nullptr); }
+int main() { return run_engine(0, {}, nullptr, nullptr); }