Skip to content

Commit cddd4a9

Browse files
authored
Avoid starting VPN twice (duckduckgo#2637)
Task/Issue URL: https://app.asana.com/0/488551667048375/1203529428649444/f ### Description #### Background Users report that AppTP causes all protected apps to lose connectivity. Reproducing this issue in-house revealed that sometimes two TUN interfaces are created. Upon further inspection, we found the root cause to be the system calling `onStartCommand` twice (even though we call `startForegroundService` once). #### Fix The fix here is to detect when we are already in the middle of creating a VPN and not start creating another one. ### Steps to test this PR - [x] Install from this branch - [x] Smoke tests for AppTP - [x] Attempt to repro the bug: 1. Open an app and then force close it 2. Disable protection for it 3. Re-enable protection 4. Rinse and repeat ~10 times - [x] Check logs for TUN errors - none should appear - [x] Check logs for the "VPN is already being started, abort" message - it should appear, meaning that we reached a condition where `onStartCommand` was called twice and the fix is working
1 parent 0341640 commit cddd4a9

File tree

1 file changed

+14
-2
lines changed

1 file changed

+14
-2
lines changed

app-tracking-protection/vpn-impl/src/main/java/com/duckduckgo/mobile/android/vpn/service/TrackerBlockingVpnService.kt

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ class TrackerBlockingVpnService : VpnService(), CoroutineScope by MainScope() {
9898

9999
private var restartRequested = false
100100

101+
private val startVpnLock = Object()
102+
101103
private val isInterceptDnsTrafficEnabled by lazy {
102104
appTpFeatureConfig.isEnabled(AppTpSetting.InterceptDnsTraffic)
103105
}
@@ -222,8 +224,18 @@ class TrackerBlockingVpnService : VpnService(), CoroutineScope by MainScope() {
222224
private suspend fun startVpn() = withContext(Dispatchers.IO) {
223225
logcat { "VPN log: Starting VPN" }
224226

225-
// We need to rethink how to log this state. This will likely change.
226-
vpnServiceStateStatsDao.insert(VpnServiceStateStats(state = ENABLING))
227+
synchronized(startVpnLock) {
228+
val currStateStats = vpnServiceStateStatsDao.getLastStateStats()
229+
if (currStateStats?.state == ENABLING) {
230+
// Sometimes onStartCommand gets called twice - this is a safety rail against that
231+
logcat(LogPriority.WARN) { "VPN is already being started, abort" }
232+
return@withContext
233+
}
234+
235+
// We need to rethink how to log this state. This will likely change.
236+
vpnServiceStateStatsDao.insert(VpnServiceStateStats(state = ENABLING))
237+
}
238+
227239
vpnNetworkStack.onPrepareVpn().getOrNull().also {
228240
if (it != null) {
229241
createTunnelInterface(it)

0 commit comments

Comments
 (0)