internal: pass engine handles throughout API (#2149)

In preparation for removing the singleton. This is split off into its own independent change to minimize the scope of the main singleton removal work. We were already doing this in many places, this just adds more instances of passing the engine handle so we do so comprehensively. Description: Pass engine handles throughout API Risk Level: Medium, changing some JNI interfaces, removing some static state Testing: Unit tests and sample apps Docs Changes: N/A internal only Release Notes: N/A internal only [Optional Fixes #Issue] [Optional Deprecated:] Signed-off-by: JP Simard [email protected] Signed-off-by: JP Simard <[email protected]>
envoyproxy · Nov 29, 2022 · 038a9bf · 038a9bf
1 parent 57fec25
commit 038a9bf
Show file tree

Hide file tree

Showing 28 changed files with 354 additions and 249 deletions.
diff --git a/mobile/library/cc/stream.cc b/mobile/library/cc/stream.cc
@@ -7,27 +7,30 @@
 namespace Envoy {
 namespace Platform {
 
-Stream::Stream(envoy_stream_t handle) : handle_(handle) {}
+Stream::Stream(envoy_engine_t engine_handle, envoy_stream_t handle)
+    : engine_handle_(engine_handle), handle_(handle) {}
 
 Stream& Stream::sendHeaders(RequestHeadersSharedPtr headers, bool end_stream) {
   envoy_headers raw_headers = rawHeaderMapAsEnvoyHeaders(headers->allHeaders());
-  ::send_headers(this->handle_, raw_headers, end_stream);
+  ::send_headers(this->engine_handle_, this->handle_, raw_headers, end_stream);
   return *this;
 }
 
 Stream& Stream::sendData(envoy_data data) {
-  ::send_data(this->handle_, data, false);
+  ::send_data(this->engine_handle_, this->handle_, data, false);
   return *this;
 }
 
 void Stream::close(RequestTrailersSharedPtr trailers) {
   envoy_headers raw_headers = rawHeaderMapAsEnvoyHeaders(trailers->allHeaders());
-  ::send_trailers(this->handle_, raw_headers);
+  ::send_trailers(this->engine_handle_, this->handle_, raw_headers);
 }
 
-void Stream::close(envoy_data data) { ::send_data(this->handle_, data, true); }
+void Stream::close(envoy_data data) {
+  ::send_data(this->engine_handle_, this->handle_, data, true);
+}
 
-void Stream::cancel() { reset_stream(this->handle_); }
+void Stream::cancel() { reset_stream(this->engine_handle_, this->handle_); }
 
 } // namespace Platform
 } // namespace Envoy
diff --git a/mobile/library/cc/stream.h b/mobile/library/cc/stream.h
@@ -15,7 +15,7 @@ using EngineSharedPtr = std::shared_ptr<Engine>;
 
 class Stream {
 public:
-  Stream(envoy_stream_t handle);
+  Stream(envoy_engine_t engine_handle, envoy_stream_t handle);
 
   Stream& sendHeaders(RequestHeadersSharedPtr headers, bool end_stream);
   Stream& sendData(envoy_data data);
@@ -24,6 +24,7 @@ class Stream {
   void cancel();
 
 private:
+  envoy_engine_t engine_handle_;
   envoy_stream_t handle_;
 };
 

diff --git a/mobile/library/cc/stream_prototype.cc b/mobile/library/cc/stream_prototype.cc
@@ -11,8 +11,9 @@ StreamPrototype::StreamPrototype(EngineSharedPtr engine) : engine_(engine) {
 
 StreamSharedPtr StreamPrototype::start() {
   auto envoy_stream = init_stream(this->engine_->engine_);
-  start_stream(envoy_stream, this->callbacks_->asEnvoyHttpCallbacks(), false);
-  return std::make_shared<Stream>(envoy_stream);
+  start_stream(this->engine_->engine_, envoy_stream, this->callbacks_->asEnvoyHttpCallbacks(),
+               false);
+  return std::make_shared<Stream>(this->engine_->engine_, envoy_stream);
 }
 
 StreamPrototype& StreamPrototype::setOnHeaders(OnHeadersCallback closure) {

diff --git a/mobile/library/common/jni/android_jni_interface.cc b/mobile/library/common/jni/android_jni_interface.cc
@@ -21,11 +21,3 @@ Java_io_envoyproxy_envoymobile_engine_AndroidJniLibrary_initialize(JNIEnv* env,
 
   return ares_library_init_android(connectivity_manager);
 }
-
-extern "C" JNIEXPORT jint JNICALL
-Java_io_envoyproxy_envoymobile_engine_AndroidJniLibrary_setPreferredNetwork(JNIEnv* env,
-                                                                            jclass, // class
-                                                                            jint network) {
-  jni_log("[Envoy]", "setting preferred network");
-  return set_preferred_network(static_cast<envoy_network_t>(network));
-}
diff --git a/mobile/library/common/jni/android_test_jni_interface.cc b/mobile/library/common/jni/android_test_jni_interface.cc
@@ -14,11 +14,3 @@ Java_io_envoyproxy_envoymobile_engine_AndroidJniLibrary_initialize(JNIEnv* env,
 
   return 0;
 }
-
-extern "C" JNIEXPORT jint JNICALL
-Java_io_envoyproxy_envoymobile_engine_AndroidJniLibrary_setPreferredNetwork(JNIEnv* env,
-                                                                            jclass, // class
-                                                                            jint network) {
-  jni_log("[Envoy]", "setting preferred network");
-  return set_preferred_network(static_cast<envoy_network_t>(network));
-}
diff --git a/mobile/library/common/jni/jni_interface.cc b/mobile/library/common/jni/jni_interface.cc
@@ -839,7 +839,8 @@ extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibr
 }
 
 extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_startStream(
-    JNIEnv* env, jclass, jlong stream_handle, jobject j_context, jboolean explicit_flow_control) {
+    JNIEnv* env, jclass, jlong engine_handle, jlong stream_handle, jobject j_context,
+    jboolean explicit_flow_control) {
 
   // TODO: To be truly safe we may need stronger guarantees of operation ordering on this ref.
   jobject retained_context = env->NewGlobalRef(j_context);
@@ -852,7 +853,8 @@ extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibra
                                            jvm_on_cancel,
                                            jvm_on_send_window_available,
                                            retained_context};
-  envoy_status_t result = start_stream(static_cast<envoy_stream_t>(stream_handle), native_callbacks,
+  envoy_status_t result = start_stream(static_cast<envoy_engine_t>(engine_handle),
+                                       static_cast<envoy_stream_t>(stream_handle), native_callbacks,
                                        explicit_flow_control);
   if (result != ENVOY_SUCCESS) {
     env->DeleteGlobalRef(retained_context); // No callbacks are fired and we need to release
@@ -934,60 +936,62 @@ Java_io_envoyproxy_envoymobile_engine_EnvoyHTTPFilterCallbacksImpl_callReleaseCa
 // EnvoyHTTPStream
 
 extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_readData(
-    JNIEnv* env, jclass, jlong stream_handle, jlong byte_count) {
-
-  return read_data(static_cast<envoy_stream_t>(stream_handle), byte_count);
+    JNIEnv* env, jclass, jlong engine_handle, jlong stream_handle, jlong byte_count) {
+  return read_data(static_cast<envoy_engine_t>(engine_handle),
+                   static_cast<envoy_stream_t>(stream_handle), byte_count);
 }
 
-// Note: JLjava_nio_ByteBuffer_2IZ is the mangled signature of the java method.
-// https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/design.html
 // The Java counterpart guarantees to invoke this method with a non-null direct ByteBuffer where the
 // provided length is between 0 and ByteBuffer.capacity(), inclusively.
-extern "C" JNIEXPORT jint JNICALL
-Java_io_envoyproxy_envoymobile_engine_JniLibrary_sendData__JLjava_nio_ByteBuffer_2IZ(
-    JNIEnv* env, jclass, jlong stream_handle, jobject data, jint length, jboolean end_stream) {
+extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_sendData(
+    JNIEnv* env, jclass, jlong engine_handle, jlong stream_handle, jobject data, jint length,
+    jboolean end_stream) {
   if (end_stream) {
     jni_log("[Envoy]", "jvm_send_data_end_stream");
   }
-
-  return send_data(static_cast<envoy_stream_t>(stream_handle),
+  return send_data(static_cast<envoy_engine_t>(engine_handle),
+                   static_cast<envoy_stream_t>(stream_handle),
                    buffer_to_native_data(env, data, length), end_stream);
 }
 
-// Note: J_3BIZ is the mangled signature of the java method.
-// https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/design.html
 // The Java counterpart guarantees to invoke this method with a non-null jbyteArray where the
 // provided length is between 0 and the size of the jbyteArray, inclusively. And given that this
 // jbyteArray comes from a ByteBuffer, it is also guaranteed that its length will not be greater
 // than 2^31 - this is why the length type is jint.
-extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_sendData__J_3BIZ(
-    JNIEnv* env, jclass, jlong stream_handle, jbyteArray data, jint length, jboolean end_stream) {
+extern "C" JNIEXPORT jint JNICALL
+Java_io_envoyproxy_envoymobile_engine_JniLibrary_sendDataByteArray(JNIEnv* env, jclass,
+                                                                   jlong engine_handle,
+                                                                   jlong stream_handle,
+                                                                   jbyteArray data, jint length,
+                                                                   jboolean end_stream) {
   if (end_stream) {
     jni_log("[Envoy]", "jvm_send_data_end_stream");
   }
-
-  return send_data(static_cast<envoy_stream_t>(stream_handle),
+  return send_data(static_cast<envoy_engine_t>(engine_handle),
+                   static_cast<envoy_stream_t>(stream_handle),
                    array_to_native_data(env, data, length), end_stream);
 }
 
 extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_sendHeaders(
-    JNIEnv* env, jclass, jlong stream_handle, jobjectArray headers, jboolean end_stream) {
-
-  return send_headers(static_cast<envoy_stream_t>(stream_handle), to_native_headers(env, headers),
+    JNIEnv* env, jclass, jlong engine_handle, jlong stream_handle, jobjectArray headers,
+    jboolean end_stream) {
+  return send_headers(static_cast<envoy_engine_t>(engine_handle),
+                      static_cast<envoy_stream_t>(stream_handle), to_native_headers(env, headers),
                       end_stream);
 }
 
 extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_sendTrailers(
-    JNIEnv* env, jclass, jlong stream_handle, jobjectArray trailers) {
+    JNIEnv* env, jclass, jlong engine_handle, jlong stream_handle, jobjectArray trailers) {
   jni_log("[Envoy]", "jvm_send_trailers");
-  return send_trailers(static_cast<envoy_stream_t>(stream_handle),
+  return send_trailers(static_cast<envoy_engine_t>(engine_handle),
+                       static_cast<envoy_stream_t>(stream_handle),
                        to_native_headers(env, trailers));
 }
 
 extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_resetStream(
-    JNIEnv* env, jclass, jlong stream_handle) {
-
-  return reset_stream(static_cast<envoy_stream_t>(stream_handle));
+    JNIEnv* env, jclass, jlong engine_handle, jlong stream_handle) {
+  return reset_stream(static_cast<envoy_engine_t>(engine_handle),
+                      static_cast<envoy_stream_t>(stream_handle));
 }
 
 // EnvoyStringAccessor
@@ -1018,3 +1022,12 @@ Java_io_envoyproxy_envoymobile_engine_JniLibrary_drainConnections(JNIEnv* env,
   jni_log("[Envoy]", "drainConnections");
   drain_connections(engine);
 }
+
+extern "C" JNIEXPORT jint JNICALL
+Java_io_envoyproxy_envoymobile_engine_JniLibrary_setPreferredNetwork(JNIEnv* env,
+                                                                     jclass, // class
+                                                                     jlong engine, jint network) {
+  jni_log("[Envoy]", "setting preferred network");
+  return set_preferred_network(static_cast<envoy_engine_t>(engine),
+                               static_cast<envoy_network_t>(network));
+}
diff --git a/mobile/library/common/main_interface.cc b/mobile/library/common/main_interface.cc
@@ -17,64 +17,65 @@ static std::atomic<envoy_stream_t> current_stream_handle_{0};
 
 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,
-                            bool explicit_flow_control) {
+envoy_status_t start_stream(envoy_engine_t engine, envoy_stream_t stream,
+                            envoy_http_callbacks callbacks, bool explicit_flow_control) {
   return Envoy::EngineHandle::runOnEngineDispatcher(
-      1 /* engine */, [stream, callbacks, explicit_flow_control](auto& engine) -> void {
+      engine, [stream, callbacks, explicit_flow_control](auto& engine) -> void {
         engine.httpClient().startStream(stream, callbacks, explicit_flow_control);
       });
 }
 
-envoy_status_t send_headers(envoy_stream_t stream, envoy_headers headers, bool end_stream) {
+envoy_status_t send_headers(envoy_engine_t engine, envoy_stream_t stream, envoy_headers headers,
+                            bool end_stream) {
   return Envoy::EngineHandle::runOnEngineDispatcher(
-      1 /* engine */, ([stream, headers, end_stream](auto& engine) -> void {
+      engine, ([stream, headers, end_stream](auto& engine) -> void {
         engine.httpClient().sendHeaders(stream, headers, end_stream);
       }));
 }
 
-envoy_status_t read_data(envoy_stream_t stream, size_t bytes_to_read) {
+envoy_status_t read_data(envoy_engine_t engine, envoy_stream_t stream, size_t bytes_to_read) {
   return Envoy::EngineHandle::runOnEngineDispatcher(
-      1 /* engine */, [stream, bytes_to_read](auto& engine) -> void {
+      engine, [stream, bytes_to_read](auto& engine) -> void {
         engine.httpClient().readData(stream, bytes_to_read);
       });
 }
 
-envoy_status_t send_data(envoy_stream_t stream, envoy_data data, bool end_stream) {
+envoy_status_t send_data(envoy_engine_t engine, envoy_stream_t stream, envoy_data data,
+                         bool end_stream) {
   return Envoy::EngineHandle::runOnEngineDispatcher(
-      1 /* engine */, [stream, data, end_stream](auto& engine) -> void {
+      engine, [stream, data, end_stream](auto& engine) -> void {
         engine.httpClient().sendData(stream, data, end_stream);
       });
 }
 
 // TODO: implement.
-envoy_status_t send_metadata(envoy_stream_t, envoy_headers) { return ENVOY_FAILURE; }
+envoy_status_t send_metadata(envoy_engine_t, envoy_stream_t, envoy_headers) {
+  return ENVOY_FAILURE;
+}
 
-envoy_status_t send_trailers(envoy_stream_t stream, envoy_headers trailers) {
+envoy_status_t send_trailers(envoy_engine_t engine, envoy_stream_t stream, envoy_headers trailers) {
   return Envoy::EngineHandle::runOnEngineDispatcher(
-      1 /* engine */, [stream, trailers](auto& engine) -> void {
+      engine, [stream, trailers](auto& engine) -> void {
         engine.httpClient().sendTrailers(stream, trailers);
       });
 }
 
-envoy_status_t reset_stream(envoy_stream_t stream) {
+envoy_status_t reset_stream(envoy_engine_t engine, envoy_stream_t stream) {
   return Envoy::EngineHandle::runOnEngineDispatcher(
-      1 /* engine */, [stream](auto& engine) -> void { engine.httpClient().cancelStream(stream); });
+      engine, [stream](auto& engine) -> void { engine.httpClient().cancelStream(stream); });
 }
 
-envoy_status_t set_preferred_network(envoy_network_t network) {
+envoy_status_t set_preferred_network(envoy_engine_t engine, envoy_network_t network) {
   envoy_netconf_t configuration_key = Envoy::Network::Configurator::setPreferredNetwork(network);
-  Envoy::EngineHandle::runOnEngineDispatcher(
-      1 /* engine */, [configuration_key](auto& engine) -> void {
-        engine.networkConfigurator().refreshDns(configuration_key);
-      });
+  Envoy::EngineHandle::runOnEngineDispatcher(engine, [configuration_key](auto& engine) -> void {
+    engine.networkConfigurator().refreshDns(configuration_key);
+  });
   // TODO(snowp): Should this return failure ever?
   return ENVOY_SUCCESS;
 }
 
 envoy_status_t record_counter_inc(envoy_engine_t e, const char* elements, envoy_stats_tags tags,
                                   uint64_t count) {
-  // TODO: use specific engine once multiple engine support is in place.
-  // https://github.com/envoyproxy/envoy-mobile/issues/332
   return Envoy::EngineHandle::runOnEngineDispatcher(
       e, [name = std::string(elements), tags, count](auto& engine) -> void {
         engine.recordCounterInc(name, tags, count);
@@ -83,8 +84,6 @@ envoy_status_t record_counter_inc(envoy_engine_t e, const char* elements, envoy_
 
 envoy_status_t record_gauge_set(envoy_engine_t e, const char* elements, envoy_stats_tags tags,
                                 uint64_t value) {
-  // TODO: use specific engine once multiple engine support is in place.
-  // https://github.com/envoyproxy/envoy-mobile/issues/332
   return Envoy::EngineHandle::runOnEngineDispatcher(
       e, [name = std::string(elements), tags, value](auto& engine) -> void {
         engine.recordGaugeSet(name, tags, value);
@@ -93,8 +92,6 @@ envoy_status_t record_gauge_set(envoy_engine_t e, const char* elements, envoy_st
 
 envoy_status_t record_gauge_add(envoy_engine_t e, const char* elements, envoy_stats_tags tags,
                                 uint64_t amount) {
-  // TODO: use specific engine once multiple engine support is in place.
-  // https://github.com/envoyproxy/envoy-mobile/issues/332
   return Envoy::EngineHandle::runOnEngineDispatcher(
       e, [name = std::string(elements), tags, amount](auto& engine) -> void {
         engine.recordGaugeAdd(name, tags, amount);
@@ -103,8 +100,6 @@ envoy_status_t record_gauge_add(envoy_engine_t e, const char* elements, envoy_st
 
 envoy_status_t record_gauge_sub(envoy_engine_t e, const char* elements, envoy_stats_tags tags,
                                 uint64_t amount) {
-  // TODO: use specific engine once multiple engine support is in place.
-  // https://github.com/envoyproxy/envoy-mobile/issues/332
   return Envoy::EngineHandle::runOnEngineDispatcher(
       e, [name = std::string(elements), tags, amount](auto& engine) -> void {
         engine.recordGaugeSub(name, tags, amount);
@@ -113,8 +108,6 @@ envoy_status_t record_gauge_sub(envoy_engine_t e, const char* elements, envoy_st
 
 envoy_status_t record_histogram_value(envoy_engine_t e, const char* elements, envoy_stats_tags tags,
                                       uint64_t value, envoy_histogram_stat_unit_t unit_measure) {
-  // TODO: use specific engine once multiple engine support is in place.
-  // https://github.com/envoyproxy/envoy-mobile/issues/332
   return Envoy::EngineHandle::runOnEngineDispatcher(
       e, [name = std::string(elements), tags, value, unit_measure](auto& engine) -> void {
         engine.recordHistogramValue(name, tags, value, unit_measure);