Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ANR due to the tokio runtime being blocked by getaddrinfo when dropped. #7210

Merged
merged 7 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ single_use_lifetimes = "warn"
unused_async = "deny"

[workspace.dependencies]
hickory-proto = "0.24.1"
hickory-resolver = "0.24.1"
hickory-server = { version = "0.24.1", features = ["resolver"] }
tokio = { version = "1.8" }
parity-tokio-ipc = "0.9"
futures = "0.3.15"
Expand Down
1 change: 1 addition & 0 deletions android/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Line wrap the file at 100 chars. Th
### Fixed
- Fix a bug where the Android account expiry notifications would not be updated if the app was
running in the background for a long time.
- Fix ANR due to the tokio runtime being blocked by `getaddrinfo` when dropped.


## [android/2024.8] - 2024-11-01
Expand Down
1 change: 1 addition & 0 deletions android/lib/talpid/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ android {
dependencies {
implementation(projects.lib.model)

implementation(libs.androidx.ktx)
implementation(libs.androidx.lifecycle.service)
implementation(libs.kermit)
implementation(libs.kotlin.stdlib)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,70 +1,79 @@
package net.mullvad.talpid

import android.content.Context
import android.net.ConnectivityManager
import android.net.ConnectivityManager.NetworkCallback
import android.net.LinkProperties
import android.net.Network
import android.net.NetworkCapabilities
import android.net.NetworkRequest
import kotlin.properties.Delegates.observable
import java.net.InetAddress
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.filterIsInstance
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.scan
import kotlinx.coroutines.flow.stateIn
import net.mullvad.talpid.util.NetworkEvent
import net.mullvad.talpid.util.defaultNetworkFlow
import net.mullvad.talpid.util.networkFlow

class ConnectivityListener {
private val availableNetworks = HashSet<Network>()
class ConnectivityListener(val connectivityManager: ConnectivityManager) {
private lateinit var _isConnected: StateFlow<Boolean>
// Used by JNI
val isConnected
get() = _isConnected.value

private val callback =
object : NetworkCallback() {
override fun onAvailable(network: Network) {
availableNetworks.add(network)
isConnected = true
}
private lateinit var _currentDnsServers: StateFlow<List<InetAddress>>
// Used by JNI
val currentDnsServers
get() = ArrayList(_currentDnsServers.value)

override fun onLost(network: Network) {
availableNetworks.remove(network)
isConnected = availableNetworks.isNotEmpty()
}
}
fun register(scope: CoroutineScope) {
_currentDnsServers =
dnsServerChanges().stateIn(scope, SharingStarted.Eagerly, currentDnsServers())

private lateinit var connectivityManager: ConnectivityManager
_isConnected =
hasInternetCapability()
.onEach { notifyConnectivityChange(it) }
.stateIn(scope, SharingStarted.Eagerly, false)
}

// Used by JNI
var isConnected by
observable(false) { _, oldValue, newValue ->
if (newValue != oldValue) {
if (senderAddress != 0L) {
notifyConnectivityChange(newValue, senderAddress)
}
}
}
private fun dnsServerChanges(): Flow<List<InetAddress>> =
connectivityManager
.defaultNetworkFlow()
.filterIsInstance<NetworkEvent.LinkPropertiesChanged>()
.map { it.linkProperties.dnsServersWithoutFallback() }

private fun currentDnsServers(): List<InetAddress> =
connectivityManager
.getLinkProperties(connectivityManager.activeNetwork)
?.dnsServersWithoutFallback() ?: emptyList()

var senderAddress = 0L
private fun LinkProperties.dnsServersWithoutFallback(): List<InetAddress> =
dnsServers.filter { it.hostAddress != TalpidVpnService.FALLBACK_DUMMY_DNS_SERVER }

fun register(context: Context) {
private fun hasInternetCapability(): Flow<Boolean> {
val request =
NetworkRequest.Builder()
.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)
.build()

connectivityManager =
context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager

connectivityManager.registerNetworkCallback(request, callback)
}

fun unregister() {
connectivityManager.unregisterNetworkCallback(callback)
}

// DROID-1401
// This function has never been used and should most likely be merged into unregister(),
// along with ensuring that the lifecycle of it is correct.
@Suppress("UnusedPrivateMember")
private fun finalize() {
destroySender(senderAddress)
senderAddress = 0L
return connectivityManager
.networkFlow(request)
.scan(setOf<Network>()) { networks, event ->
when (event) {
is NetworkEvent.Available -> networks + event.network
is NetworkEvent.Lost -> networks - event.network
else -> networks
}
}
.map { it.isNotEmpty() }
.distinctUntilChanged()
}

private external fun notifyConnectivityChange(isConnected: Boolean, senderAddress: Long)

private external fun destroySender(senderAddress: Long)
private external fun notifyConnectivityChange(isConnected: Boolean)
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package net.mullvad.talpid

import android.net.ConnectivityManager
import android.os.ParcelFileDescriptor
import androidx.annotation.CallSuper
import androidx.core.content.getSystemService
import androidx.lifecycle.lifecycleScope
import co.touchlab.kermit.Logger
import java.net.Inet4Address
import java.net.Inet6Address
Expand Down Expand Up @@ -29,18 +32,13 @@ open class TalpidVpnService : LifecycleVpnService() {
private var currentTunConfig: TunConfig? = null

// Used by JNI
val connectivityListener = ConnectivityListener()
lateinit var connectivityListener: ConnectivityListener

@CallSuper
override fun onCreate() {
super.onCreate()
connectivityListener.register(this)
}

@CallSuper
override fun onDestroy() {
super.onDestroy()
connectivityListener.unregister()
connectivityListener = ConnectivityListener(getSystemService<ConnectivityManager>()!!)
connectivityListener.register(lifecycleScope)
}

fun openTun(config: TunConfig): CreateTunResult {
Expand Down Expand Up @@ -161,7 +159,7 @@ open class TalpidVpnService : LifecycleVpnService() {
private external fun waitForTunnelUp(tunFd: Int, isIpv6Enabled: Boolean)

companion object {
private const val FALLBACK_DUMMY_DNS_SERVER = "192.0.2.1"
const val FALLBACK_DUMMY_DNS_SERVER = "192.0.2.1"

private const val IPV4_PREFIX_LENGTH = 32
private const val IPV6_PREFIX_LENGTH = 128
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
package net.mullvad.talpid.util

import android.net.ConnectivityManager
import android.net.ConnectivityManager.NetworkCallback
import android.net.LinkProperties
import android.net.Network
import android.net.NetworkCapabilities
import android.net.NetworkRequest
import kotlinx.coroutines.channels.awaitClose
import kotlinx.coroutines.channels.trySendBlocking
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.callbackFlow

fun ConnectivityManager.defaultNetworkFlow(): Flow<NetworkEvent> =
callbackFlow<NetworkEvent> {
val callback =
object : NetworkCallback() {
override fun onLinkPropertiesChanged(
network: Network,
linkProperties: LinkProperties,
) {
super.onLinkPropertiesChanged(network, linkProperties)
trySendBlocking(NetworkEvent.LinkPropertiesChanged(network, linkProperties))
}

override fun onAvailable(network: Network) {
super.onAvailable(network)
trySendBlocking(NetworkEvent.Available(network))
}

override fun onCapabilitiesChanged(
network: Network,
networkCapabilities: NetworkCapabilities,
) {
super.onCapabilitiesChanged(network, networkCapabilities)
trySendBlocking(NetworkEvent.CapabilitiesChanged(network, networkCapabilities))
}

override fun onBlockedStatusChanged(network: Network, blocked: Boolean) {
super.onBlockedStatusChanged(network, blocked)
trySendBlocking(NetworkEvent.BlockedStatusChanged(network, blocked))
}

override fun onLosing(network: Network, maxMsToLive: Int) {
super.onLosing(network, maxMsToLive)
trySendBlocking(NetworkEvent.Losing(network, maxMsToLive))
}

override fun onLost(network: Network) {
super.onLost(network)
trySendBlocking(NetworkEvent.Lost(network))
}

override fun onUnavailable() {
super.onUnavailable()
trySendBlocking(NetworkEvent.Unavailable)
}
}
registerDefaultNetworkCallback(callback)

awaitClose { unregisterNetworkCallback(callback) }
}

fun ConnectivityManager.networkFlow(networkRequest: NetworkRequest): Flow<NetworkEvent> =
callbackFlow<NetworkEvent> {
val callback =
object : NetworkCallback() {
override fun onLinkPropertiesChanged(
network: Network,
linkProperties: LinkProperties,
) {
super.onLinkPropertiesChanged(network, linkProperties)
trySendBlocking(NetworkEvent.LinkPropertiesChanged(network, linkProperties))
}

override fun onAvailable(network: Network) {
super.onAvailable(network)
trySendBlocking(NetworkEvent.Available(network))
}

override fun onCapabilitiesChanged(
network: Network,
networkCapabilities: NetworkCapabilities,
) {
super.onCapabilitiesChanged(network, networkCapabilities)
trySendBlocking(NetworkEvent.CapabilitiesChanged(network, networkCapabilities))
}

override fun onBlockedStatusChanged(network: Network, blocked: Boolean) {
super.onBlockedStatusChanged(network, blocked)
trySendBlocking(NetworkEvent.BlockedStatusChanged(network, blocked))
}

override fun onLosing(network: Network, maxMsToLive: Int) {
super.onLosing(network, maxMsToLive)
trySendBlocking(NetworkEvent.Losing(network, maxMsToLive))
}

override fun onLost(network: Network) {
super.onLost(network)
trySendBlocking(NetworkEvent.Lost(network))
}

override fun onUnavailable() {
super.onUnavailable()
trySendBlocking(NetworkEvent.Unavailable)
}
}
registerNetworkCallback(networkRequest, callback)

awaitClose { unregisterNetworkCallback(callback) }
}

sealed interface NetworkEvent {
data class Available(val network: Network) : NetworkEvent

data object Unavailable : NetworkEvent

data class LinkPropertiesChanged(val network: Network, val linkProperties: LinkProperties) :
NetworkEvent

data class CapabilitiesChanged(
val network: Network,
val networkCapabilities: NetworkCapabilities,
) : NetworkEvent

data class BlockedStatusChanged(val network: Network, val blocked: Boolean) : NetworkEvent

data class Losing(val network: Network, val maxMsToLive: Int) : NetworkEvent

data class Lost(val network: Network) : NetworkEvent
}
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ class MullvadVpnService : TalpidVpnService() {
}

override fun onDestroy() {
super.onDestroy()
Logger.i("MullvadVpnService: onDestroy")
// Shutting down the daemon gracefully
managementService.stop()
Expand All @@ -214,7 +215,6 @@ class MullvadVpnService : TalpidVpnService() {
managementService.enterIdle()

Logger.i("Shutdown complete")
super.onDestroy()
}

// If an intent is from the system it is because of the OS starting/stopping the VPN.
Expand Down
1 change: 1 addition & 0 deletions mullvad-api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ workspace = true
api-override = []

[dependencies]
async-trait = "0.1"
libc = "0.2"
chrono = { workspace = true }
thiserror = { workspace = true }
Expand Down
6 changes: 4 additions & 2 deletions mullvad-api/src/bin/relay_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@
//! Used by the installer artifact packer to bundle the latest available
//! relay list at the time of creating the installer.

use mullvad_api::{proxy::ApiConnectionMode, rest::Error as RestError, RelayListProxy};
use mullvad_api::{
proxy::ApiConnectionMode, rest::Error as RestError, DefaultDnsResolver, RelayListProxy,
};
use std::process;
use talpid_types::ErrorExt;

#[tokio::main]
async fn main() {
let runtime = mullvad_api::Runtime::new(tokio::runtime::Handle::current())
let runtime = mullvad_api::Runtime::new(tokio::runtime::Handle::current(), DefaultDnsResolver)
.expect("Failed to load runtime");

let relay_list_request =
Expand Down
Loading
Loading