Skip to content

Commit 5c6705d

Browse files
nshubaaitorvs
authored andcommitted
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 9536f97 commit 5c6705d

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
@@ -100,6 +100,8 @@ class TrackerBlockingVpnService : VpnService(), CoroutineScope by MainScope() {
100100

101101
private var restartRequested = false
102102

103+
private val startVpnLock = Object()
104+
103105
private val isInterceptDnsTrafficEnabled by lazy {
104106
appTpFeatureConfig.isEnabled(AppTpSetting.InterceptDnsTraffic)
105107
}
@@ -217,8 +219,18 @@ class TrackerBlockingVpnService : VpnService(), CoroutineScope by MainScope() {
217219
private suspend fun startVpn() = withContext(Dispatchers.IO) {
218220
Timber.d("VPN log: Starting VPN")
219221

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

0 commit comments

Comments
 (0)