diff --git a/mobile/library/java/io/envoyproxy/envoymobile/engine/AndroidNetworkMonitor.java b/mobile/library/java/io/envoyproxy/envoymobile/engine/AndroidNetworkMonitor.java index fc10cddc384f..af80b1867b92 100644 --- a/mobile/library/java/io/envoyproxy/envoymobile/engine/AndroidNetworkMonitor.java +++ b/mobile/library/java/io/envoyproxy/envoymobile/engine/AndroidNetworkMonitor.java @@ -55,6 +55,15 @@ public static void load(Context context, EnvoyEngine envoyEngine) { } } + /** + * Sets the {@link AndroidNetworkMonitor} singleton instance to null, so that it can be recreated + * when a new EnvoyEngine is created. + */ + @VisibleForTesting + public static void shutdown() { + instance = null; + } + private AndroidNetworkMonitor(Context context, EnvoyEngine envoyEngine) { int permission = ContextCompat.checkSelfPermission(context, Manifest.permission.ACCESS_NETWORK_STATE); diff --git a/mobile/library/jni/jni_impl.cc b/mobile/library/jni/jni_impl.cc index 6a25e6c8cc83..67aaba6140b3 100644 --- a/mobile/library/jni/jni_impl.cc +++ b/mobile/library/jni/jni_impl.cc @@ -36,27 +36,23 @@ Java_io_envoyproxy_envoymobile_engine_JniLibrary_setLogLevel(JNIEnv* /*env*/, jc } extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_initEngine( - JNIEnv* env, jclass, jobject on_start_context, jobject envoy_logger_context, - jobject event_tracker_context) { + JNIEnv* env, jclass, jobject on_engine_running, jobject envoy_logger, + jobject envoy_event_tracker) { //================================================================================================ // EngineCallbacks //================================================================================================ std::unique_ptr callbacks = std::make_unique(); - if (on_start_context != nullptr) { - jobject retained_on_start_context = - env->NewGlobalRef(on_start_context); // Required to keep context in memory - callbacks->on_engine_running_ = [retained_on_start_context] { + if (on_engine_running != nullptr) { + jobject on_engine_running_global_ref = env->NewGlobalRef(on_engine_running); + callbacks->on_engine_running_ = [on_engine_running_global_ref] { Envoy::JNI::JniHelper jni_helper(Envoy::JNI::getEnv()); Envoy::JNI::LocalRefUniquePtr java_on_engine_running_class = - jni_helper.getObjectClass(retained_on_start_context); + jni_helper.getObjectClass(on_engine_running_global_ref); jmethodID java_on_engine_running_method_id = jni_helper.getMethodId( java_on_engine_running_class.get(), "invokeOnEngineRunning", "()Ljava/lang/Object;"); - Envoy::JNI::LocalRefUniquePtr unused = - jni_helper.callObjectMethod(retained_on_start_context, java_on_engine_running_method_id); - - // TODO(goaway): This isn't re-used by other engine callbacks, so it's safe to delete here. - // This will need to be updated for https://github.com/envoyproxy/envoy-mobile/issues/332 - jni_helper.getEnv()->DeleteGlobalRef(retained_on_start_context); + Envoy::JNI::LocalRefUniquePtr unused = jni_helper.callObjectMethod( + on_engine_running_global_ref, java_on_engine_running_method_id); + jni_helper.getEnv()->DeleteGlobalRef(on_engine_running_global_ref); }; callbacks->on_exit_ = [] { // Note that this is not dispatched because the thread that @@ -70,23 +66,23 @@ extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibr // EnvoyLogger //================================================================================================ std::unique_ptr logger = std::make_unique(); - if (envoy_logger_context != nullptr) { - const jobject retained_logger_context = env->NewGlobalRef(envoy_logger_context); - logger->on_log_ = [retained_logger_context](Envoy::Logger::Logger::Levels level, + if (envoy_logger != nullptr) { + const jobject envoy_logger_global_ref = env->NewGlobalRef(envoy_logger); + logger->on_log_ = [envoy_logger_global_ref](Envoy::Logger::Logger::Levels level, const std::string& message) { Envoy::JNI::JniHelper jni_helper(Envoy::JNI::getEnv()); Envoy::JNI::LocalRefUniquePtr java_message = - jni_helper.newStringUtf(message.c_str()); + Envoy::JNI::cppStringToJavaString(jni_helper, message); jint java_level = static_cast(level); Envoy::JNI::LocalRefUniquePtr java_envoy_logger_class = - jni_helper.getObjectClass(retained_logger_context); + jni_helper.getObjectClass(envoy_logger_global_ref); jmethodID java_log_method_id = jni_helper.getMethodId(java_envoy_logger_class.get(), "log", "(ILjava/lang/String;)V"); - jni_helper.callVoidMethod(retained_logger_context, java_log_method_id, java_level, + jni_helper.callVoidMethod(envoy_logger_global_ref, java_log_method_id, java_level, java_message.get()); }; - logger->on_exit_ = [retained_logger_context] { - Envoy::JNI::getEnv()->DeleteGlobalRef(retained_logger_context); + logger->on_exit_ = [envoy_logger_global_ref] { + Envoy::JNI::getEnv()->DeleteGlobalRef(envoy_logger_global_ref); }; } //================================================================================================ @@ -94,26 +90,25 @@ extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibr //================================================================================================ std::unique_ptr event_tracker = std::make_unique(); - if (event_tracker_context != nullptr) { - const jobject retained_event_tracker_context = env->NewGlobalRef(event_tracker_context); - event_tracker->on_track_ = [retained_event_tracker_context]( + if (envoy_event_tracker != nullptr) { + const jobject event_tracker_global_ref = env->NewGlobalRef(envoy_event_tracker); + event_tracker->on_track_ = [event_tracker_global_ref]( const absl::flat_hash_map& events) { Envoy::JNI::JniHelper jni_helper(Envoy::JNI::getEnv()); Envoy::JNI::LocalRefUniquePtr java_events = Envoy::JNI::cppMapToJavaMap(jni_helper, events); Envoy::JNI::LocalRefUniquePtr java_envoy_event_tracker_class = - jni_helper.getObjectClass(retained_event_tracker_context); + jni_helper.getObjectClass(event_tracker_global_ref); jmethodID java_track_method_id = jni_helper.getMethodId(java_envoy_event_tracker_class.get(), "track", "(Ljava/util/Map;)V"); - jni_helper.callVoidMethod(retained_event_tracker_context, java_track_method_id, - java_events.get()); + jni_helper.callVoidMethod(event_tracker_global_ref, java_track_method_id, java_events.get()); }; - event_tracker->on_exit_ = [retained_event_tracker_context] { - Envoy::JNI::getEnv()->DeleteGlobalRef(retained_event_tracker_context); + event_tracker->on_exit_ = [event_tracker_global_ref] { + Envoy::JNI::getEnv()->DeleteGlobalRef(event_tracker_global_ref); }; } - return reinterpret_cast( + return reinterpret_cast( new Envoy::InternalEngine(std::move(callbacks), std::move(logger), std::move(event_tracker))); } @@ -144,7 +139,9 @@ extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibra extern "C" JNIEXPORT void JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_terminateEngine( JNIEnv* /*env*/, jclass, jlong engine_handle) { - reinterpret_cast(engine_handle)->terminate(); + Envoy::InternalEngine* internal_engine = reinterpret_cast(engine_handle); + internal_engine->terminate(); + delete internal_engine; } extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_recordCounterInc( diff --git a/mobile/test/java/org/chromium/net/CronetUrlRequestTest.java b/mobile/test/java/org/chromium/net/CronetUrlRequestTest.java index 1c984c747df1..e5f0aa1f379c 100644 --- a/mobile/test/java/org/chromium/net/CronetUrlRequestTest.java +++ b/mobile/test/java/org/chromium/net/CronetUrlRequestTest.java @@ -35,11 +35,11 @@ import java.util.regex.Pattern; import io.envoyproxy.envoymobile.engine.AndroidNetworkMonitor; import org.chromium.net.impl.CronvoyUrlRequest; +import org.chromium.net.impl.CronvoyUrlRequestContext; import org.chromium.net.impl.Errors.EnvoyMobileError; import org.chromium.net.impl.Errors.NetError; import org.chromium.net.impl.CronvoyUrlResponseInfoImpl; import org.chromium.net.testing.CronetTestRule; -import org.chromium.net.testing.CronetTestRule.CronetTestFramework; import org.chromium.net.testing.CronetTestRule.RequiresMinApi; import org.chromium.net.testing.FailurePhase; import org.chromium.net.testing.Feature; @@ -77,7 +77,6 @@ public class CronetUrlRequestTest { @Rule public final RuleChain chain = RuleChain.outerRule(mTestRule).around(mRuntimePermissionRule); - private CronetTestFramework mTestFramework; private MockUrlRequestJobFactory mMockUrlRequestJobFactory; @Before @@ -91,6 +90,11 @@ public void setUp() { public void tearDown() { mMockUrlRequestJobFactory.shutdown(); NativeTestServer.shutdownNativeTestServer(); + // Calling AndroidNetworkMonitor.shutdown() will set the AndroidNetworkMonitor singleton + // instance to null so that the next EnvoyEngine creation will have a new AndroidNetworkMonitor + // instance instead of holding on a dangling EnvoyEngine because AndroidNetworkMonitor.load + // does not update the singleton instance to a new one if there is already an existing instance. + AndroidNetworkMonitor.shutdown(); } private TestUrlRequestCallback startAndWaitForComplete(CronetEngine engine, String url)