Skip to content

Commit

Permalink
Merge branch 'toggletunnel-crash-in-tile-droid-1283'
Browse files Browse the repository at this point in the history
  • Loading branch information
Rawa committed Aug 30, 2024
2 parents b0c1bcc + 370e49b commit 6274589
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import arrow.atomic.AtomicInt
import co.touchlab.kermit.Logger
import java.io.File
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.filterIsInstance
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import net.mullvad.mullvadvpn.lib.common.constant.KEY_CONNECT_ACTION
Expand Down Expand Up @@ -90,6 +90,15 @@ class MullvadVpnService : TalpidVpnService() {

Logger.i("Start management service")
managementService.start()

lifecycleScope.launch {
// If the service is started with a connect command and a non-blocking error occur (e.g.
// unable to start the tunnel) then the service is demoted from foreground.
managementService.tunnelState
.filterIsInstance<TunnelState.Error>()
.filter { !it.errorState.isBlocking }
.collect { foregroundNotificationHandler.stopForeground() }
}
}

override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {
Expand All @@ -105,15 +114,24 @@ class MullvadVpnService : TalpidVpnService() {
keyguardManager.isKeyguardLocked -> {
Logger.i("Keyguard is locked, ignoring command")
}

intent.isFromSystem() || intent?.action == KEY_CONNECT_ACTION -> {
// Only show on foreground if we have permission
if (prepare(this) == null) {
foregroundNotificationHandler.startForeground()
}
foregroundNotificationHandler.startForeground()
lifecycleScope.launch { connectionProxy.connectWithoutPermissionCheck() }
}

intent?.action == KEY_DISCONNECT_ACTION -> {
// MullvadTileService might have launched this service with the expectancy of it
// being foreground, thus it must go into foreground to please the android system
// requirements.
foregroundNotificationHandler.startForeground()
lifecycleScope.launch { connectionProxy.disconnect() }

// If disconnect intent is received and no one is using this service, simply stop
// foreground and let system stop service when it deems it not to be necessary.
if (bindCount.get() == 0) {
foregroundNotificationHandler.stopForeground()
}
}
}

Expand Down Expand Up @@ -180,25 +198,6 @@ class MullvadVpnService : TalpidVpnService() {
foregroundNotificationHandler.stopForeground()
}

if (count == 0) {
Logger.i("No one bound to the service, stopSelf()")
lifecycleScope.launch {
Logger.i("Waiting for disconnected state")
// TODO This needs reworking, we should not wait for the disconnected state, what we
// want is the notification of disconnected to go out before we start shutting down
connectionProxy.tunnelState
.filter {
it is TunnelState.Disconnected ||
(it is TunnelState.Error && !it.errorState.isBlocking)
}
.first()

if (bindCount.get() == 0) {
Logger.i("Stopping service")
stopSelf()
}
}
}
return true
}

Expand All @@ -209,8 +208,10 @@ class MullvadVpnService : TalpidVpnService() {

Logger.i("Shutdown MullvadDaemon")
MullvadDaemon.shutdown()

Logger.i("Enter Idle")
managementService.enterIdle()

Logger.i("Shutdown complete")
super.onDestroy()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@ import android.content.Context
import android.content.pm.PackageManager
import androidx.core.app.ActivityCompat
import androidx.core.app.NotificationManagerCompat
import kotlin.time.Duration.Companion.milliseconds
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.FlowPreview
import kotlinx.coroutines.flow.debounce
import kotlinx.coroutines.flow.merge
import kotlinx.coroutines.launch
import net.mullvad.mullvadvpn.lib.model.Notification
import net.mullvad.mullvadvpn.lib.model.NotificationUpdate
import net.mullvad.mullvadvpn.service.notifications.accountexpiry.toNotification
import net.mullvad.mullvadvpn.service.notifications.tunnelstate.toNotification

@OptIn(FlowPreview::class)
class NotificationManager(
private val notificationManagerCompat: NotificationManagerCompat,
notificationProviders: List<NotificationProvider<Notification>>,
Expand All @@ -23,14 +27,15 @@ class NotificationManager(
init {
scope.launch {
notificationProviders
.map { it.notifications }
.map { it.notifications.debounce(NOTIFICATION_DEBOUNCE) }
.merge()
.collect { notificationUpdate ->
when (notificationUpdate) {
is NotificationUpdate.Cancel ->
notificationManagerCompat.cancel(
notificationUpdate.notificationId.value
)

is NotificationUpdate.Notify -> {
val notification = notificationUpdate.value
val androidNotification = notification.toAndroidNotification(context)
Expand All @@ -56,4 +61,11 @@ class NotificationManager(
is Notification.Tunnel -> toNotification(context)
is Notification.AccountExpiry -> toNotification(context)
}

companion object {
// According to testing we are only allowed to send 5 notifications per second at most,
// otherwise the system will start dropping them. To ensure we don't drop the latest
// notification debounce if we spam too much.
val NOTIFICATION_DEBOUNCE = 200.milliseconds
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ class TunnelStateNotificationProvider(
connectionProxy.tunnelState,
connectionProxy.tunnelState.actionAfterDisconnect().distinctUntilChanged(),
deviceRepository.deviceState,
) { tunnelState: TunnelState, actionAfterDisconnect: ActionAfterDisconnect?, deviceState
->
) { tunnelState, actionAfterDisconnect, deviceState ->
if (
deviceState is DeviceState.LoggedOut && tunnelState is TunnelState.Disconnected
) {
Expand Down Expand Up @@ -113,7 +112,8 @@ class TunnelStateNotificationProvider(
): NotificationTunnelState.Error {
val cause = errorState.cause
return when {
cause is ErrorStateCause.IsOffline -> NotificationTunnelState.Error.DeviceOffline
cause is ErrorStateCause.IsOffline && errorState.isBlocking ->
NotificationTunnelState.Error.DeviceOffline
cause is ErrorStateCause.InvalidDnsServers -> NotificationTunnelState.Error.Blocking
cause is ErrorStateCause.VpnPermissionDenied ->
alwaysOnVpnPermissionName?.let { NotificationTunnelState.Error.AlwaysOnVpn }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ class MullvadTileService : TileService() {
}
}

// Always start as foreground in case tile is out-of-sync.
// Always start as foreground, e.g if app is dead we won't be allowed to start if not
// in foreground.
startForegroundService(intent)
}

Expand Down

0 comments on commit 6274589

Please sign in to comment.