From 12e8251fcdd22fca588adb76cc54f8f1776c8293 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Thu, 21 Nov 2024 14:56:30 +0100 Subject: [PATCH] Make connectivity sender static --- .../mullvad/talpid/ConnectivityListener.kt | 23 +-- .../net/mullvad/talpid/TalpidVpnService.kt | 6 - talpid-core/src/connectivity_listener.rs | 137 +++++------------- talpid-core/src/offline/android.rs | 2 +- 4 files changed, 41 insertions(+), 127 deletions(-) diff --git a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt index a37cf18578df..4cb67f9945e6 100644 --- a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt +++ b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt @@ -21,15 +21,6 @@ import net.mullvad.talpid.util.defaultNetworkFlow import net.mullvad.talpid.util.networkFlow class ConnectivityListener(val connectivityManager: ConnectivityManager) { - // Used by JNI - var senderAddress = 0L - set(value) { - if (value == 0L) { - destroySender(field) - } - field = value - } - private lateinit var _isConnected: StateFlow // Used by JNI val isConnected @@ -46,18 +37,10 @@ class ConnectivityListener(val connectivityManager: ConnectivityManager) { _isConnected = hasInternetCapability() - .onEach { - if (senderAddress != 0L) { - notifyConnectivityChange(it, senderAddress) - } - } + .onEach { notifyConnectivityChange(it) } .stateIn(scope, SharingStarted.Eagerly, false) } - fun unregister() { - senderAddress = 0L - } - private fun dnsServerChanges(): Flow> = connectivityManager .defaultNetworkFlow() @@ -92,7 +75,5 @@ class ConnectivityListener(val connectivityManager: ConnectivityManager) { .distinctUntilChanged() } - private external fun notifyConnectivityChange(isConnected: Boolean, senderAddress: Long) - - private external fun destroySender(senderAddress: Long) + private external fun notifyConnectivityChange(isConnected: Boolean) } diff --git a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt index dfd6699b1e33..dc1f8d23ca90 100644 --- a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt +++ b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt @@ -41,12 +41,6 @@ open class TalpidVpnService : LifecycleVpnService() { connectivityListener.register(lifecycleScope) } - @CallSuper - override fun onDestroy() { - super.onDestroy() - connectivityListener.unregister() - } - fun openTun(config: TunConfig): CreateTunResult { synchronized(this) { val tunStatus = activeTunStatus diff --git a/talpid-core/src/connectivity_listener.rs b/talpid-core/src/connectivity_listener.rs index 2d121d264bcf..9bdf4bf87ab7 100644 --- a/talpid-core/src/connectivity_listener.rs +++ b/talpid-core/src/connectivity_listener.rs @@ -5,13 +5,15 @@ use jnix::{ jni::{ self, objects::{GlobalRef, JObject, JValue}, - signature::{JavaType, Primitive}, - sys::{jboolean, jlong, JNI_TRUE}, + sys::{jboolean, JNI_TRUE}, JNIEnv, JavaVM, }, FromJava, JnixEnv, }; -use std::{net::IpAddr, sync::Arc}; +use std::{ + net::IpAddr, + sync::{Arc, Mutex}, +}; use talpid_types::{android::AndroidContext, net::Connectivity, ErrorExt}; /// Error related to Android connectivity monitor @@ -42,10 +44,11 @@ pub enum Error { #[derive(Clone)] pub struct ConnectivityListener { jvm: Arc, - class: GlobalRef, - object: GlobalRef, + android_listener: GlobalRef, } +static CONNECTIVITY_TX: Mutex>> = Mutex::new(None); + impl ConnectivityListener { /// Create a new connectivity listener pub fn new(android_context: AndroidContext) -> Result { @@ -56,28 +59,18 @@ impl ConnectivityListener { .map_err(Error::AttachJvmToThread)?, ); - let get_connectivity_listener_method = env - .get_method_id( - &env.get_class("net/mullvad/talpid/TalpidVpnService"), - "getConnectivityListener", - "()Lnet/mullvad/talpid/ConnectivityListener;", - ) - .map_err(|cause| { - Error::FindMethod("MullvadVpnService", "getConnectivityListener", cause) - })?; - let result = env - .call_method_unchecked( + .call_method( android_context.vpn_service.as_obj(), - get_connectivity_listener_method, - JavaType::Object("Lnet/mullvad/talpid/ConnectivityListener;".to_owned()), + "getConnectivityListener", + "()Lnet/mullvad/talpid/ConnectivityListener;", &[], ) .map_err(|cause| { Error::CallMethod("MullvadVpnService", "getConnectivityListener", cause) })?; - let object = match result { + let android_listener = match result { JValue::Object(object) => env.new_global_ref(object).map_err(Error::CreateGlobalRef)?, value => { return Err(Error::InvalidMethodResult( @@ -88,43 +81,19 @@ impl ConnectivityListener { } }; - let class = env.get_class("net/mullvad/talpid/ConnectivityListener"); - Ok(ConnectivityListener { jvm: android_context.jvm, - class, - object, + android_listener, }) } - /// Register a channel that receives changes about the offline state + /// Register a channel that receives changes about the offline state. /// /// # Note /// /// The listener is shared by all instances of the struct. - pub fn set_connectivity_listener( - &mut self, - sender: UnboundedSender, - ) -> Result<(), Error> { - let sender_ptr = Box::into_raw(Box::new(sender)) as jlong; - - let result = self.call_method( - "setSenderAddress", - "(J)V", - &[JValue::Long(sender_ptr)], - JavaType::Primitive(Primitive::Void), - )?; - - match result { - JValue::Void => Ok(()), - value => Err(Error::InvalidMethodResult( - "ConnectivityListener", - "setSenderAddress", - format!("{:?}", value), - )), - }?; - - Ok(()) + pub fn set_connectivity_listener(&mut self, sender: UnboundedSender) { + *CONNECTIVITY_TX.lock().unwrap() = Some(sender); } /// Return the current offline/connectivity state @@ -141,16 +110,18 @@ impl ConnectivityListener { } fn get_is_connected(&self) -> Result { - let is_connected = self.call_method( - "isConnected", - "()Z", - &[], - JavaType::Primitive(Primitive::Boolean), - )?; + let env = JnixEnv::from( + self.jvm + .attach_current_thread_as_daemon() + .map_err(Error::AttachJvmToThread)?, + ); + + let is_connected = + env.call_method(self.android_listener.as_obj(), "isConnected", "()Z", &[]); match is_connected { - JValue::Bool(JNI_TRUE) => Ok(true), - JValue::Bool(_) => Ok(false), + Ok(JValue::Bool(JNI_TRUE)) => Ok(true), + Ok(JValue::Bool(_)) => Ok(false), value => Err(Error::InvalidMethodResult( "ConnectivityListener", "isConnected", @@ -167,43 +138,22 @@ impl ConnectivityListener { .map_err(Error::AttachJvmToThread)?, ); - let current_dns_servers = self.call_method( + let current_dns_servers = env.call_method( + self.android_listener.as_obj(), "getCurrentDnsServers", "()Ljava/util/ArrayList;", &[], - JavaType::Object("java/util/ArrayList".to_owned()), - )?; + ); match current_dns_servers { - JValue::Object(jaddrs) => Ok(Vec::from_java(&env, jaddrs)), + Ok(JValue::Object(jaddrs)) => Ok(Vec::from_java(&env, jaddrs)), value => Err(Error::InvalidMethodResult( "ConnectivityListener", - "currentDnsServers", + "getCurrentDnsServers", format!("{:?}", value), )), } } - - fn call_method( - &self, - method: &'static str, - signature: &str, - parameters: &[JValue<'_>], - return_type: JavaType, - ) -> Result, Error> { - let env = JnixEnv::from( - self.jvm - .attach_current_thread_as_daemon() - .map_err(Error::AttachJvmToThread)?, - ); - - let method_id = env - .get_method_id(&self.class, method, signature) - .map_err(|cause| Error::FindMethod("ConnectivityListener", method, cause))?; - - env.call_method_unchecked(self.object.as_obj(), method_id, return_type, parameters) - .map_err(|cause| Error::CallMethod("ConnectivityListener", method, cause)) - } } /// Entry point for Android Java code to notify the connectivity status. @@ -213,30 +163,19 @@ pub extern "system" fn Java_net_mullvad_talpid_ConnectivityListener_notifyConnec _: JNIEnv<'_>, _: JObject<'_>, connected: jboolean, - sender_address: jlong, ) { - let connected = JNI_TRUE == connected; + let Some(tx) = &*CONNECTIVITY_TX.lock().unwrap() else { + // No sender has been registered + log::trace!("Received connectivity notification wíth no channel"); + return; + }; - let sender = unsafe { Box::from_raw(sender_address as *mut UnboundedSender) }; + let connected = JNI_TRUE == connected; - if sender + if tx .unbounded_send(Connectivity::Status { connected }) .is_err() { log::warn!("Failed to send offline change event"); } - - // Do not destroy - std::mem::forget(sender); -} - -/// Entry point for Android Java code to return ownership of the sender reference. -#[no_mangle] -#[allow(non_snake_case)] -pub extern "system" fn Java_net_mullvad_talpid_ConnectivityListener_destroySender( - _: JNIEnv<'_>, - _: JObject<'_>, - sender_address: jlong, -) { - let _ = unsafe { Box::from_raw(sender_address as *mut UnboundedSender) }; } diff --git a/talpid-core/src/offline/android.rs b/talpid-core/src/offline/android.rs index 7280ee792f09..4947c7f61ebd 100644 --- a/talpid-core/src/offline/android.rs +++ b/talpid-core/src/offline/android.rs @@ -27,6 +27,6 @@ pub async fn spawn_monitor( let mut monitor_handle = MonitorHandle::new(connectivity_listener); monitor_handle .connectivity_listener - .set_connectivity_listener(sender)?; + .set_connectivity_listener(sender); Ok(monitor_handle) }