Notification: gate changes. (#9087)

Co-authored-by: atavism <atavism@users.noreply.github.com>

Author: jigar-f

android/app/src/main/kotlin/org/getlantern/lantern/notification/NotificationManager.kt

@@ -159,13 +159,16 @@ class NotificationHelper {
     /**
      * Shows the starting VPN notification as a foreground notification.
      * Also starts the service in the foreground and promotes it to a foreground service.
+     *
+     * @return true if this call promoted the service; false if it was already in the foreground.
      */
     @Synchronized
-    fun showStartingVPNConnectedNotification(vpnService: LanternVpnService) {
+    fun showStartingVPNConnectedNotification(vpnService: LanternVpnService): Boolean {
         // Duplicate starts must not replace an existing connected notification.
-        if (foregroundStarted) return
+        if (foregroundStarted) return false
         showForegroundNotification(vpnService, VPN_CONNECTED, buildStartingVpnNotification())
         foregroundStarted = true
+        return true
     }
 
     /**

android/app/src/main/kotlin/org/getlantern/lantern/service/LanternVpnService.kt

@@ -349,15 +349,27 @@ class LanternVpnService :
     ) = withContext(Dispatchers.IO) {
         // A duplicate may also arrive via startForegroundService, so promote the
         // service before deciding which request owns startup.
+        var promotedHere = false
         val foregroundFailure = try {
-            notificationHelper.showStartingVPNConnectedNotification(this@LanternVpnService)
+            promotedHere = notificationHelper.showStartingVPNConnectedNotification(this@LanternVpnService)
             null
         } catch (e: CancellationException) {
             throw e
         } catch (e: Exception) {
             e
         }
-        val accepted = vpnStartGate.run(waitForIdle = restart) { attempt ->
+        val accepted = vpnStartGate.run(
+            waitForIdle = restart,
+            onRejected = { rejection ->
+                AppLogger.i(TAG, "VPN operation ($errorCode) ignored: $rejection")
+                // Another start owns the notification; a stop leaves nobody to clear
+                // a promotion made here (e.g. a fresh instance during teardown).
+                if (rejection == VpnStartGate.Rejection.STOPPING && promotedHere) {
+                    notificationHelper.stopVPNConnectedNotification(this@LanternVpnService)
+                    stopSelf()
+                }
+            },
+        ) { attempt ->
             try {
                 if (prepare(this@LanternVpnService) != null) {
                     attempt.publish { VpnStatusManager.postVPNStatus(VPNStatus.MissingPermission) }
@@ -410,9 +422,6 @@ class LanternVpnService :
                 if (e is CancellationException) throw e
             }
         }
-        if (!accepted) {
-            AppLogger.i(TAG, "VPN operation ($errorCode) ignored: another start or stop is in progress")
-        }
         accepted
     }
 

android/app/src/main/kotlin/org/getlantern/lantern/service/VpnStartGate.kt

@@ -26,26 +26,39 @@ internal class VpnStartGate(
     private var pendingStops = 0
     private var stopGeneration = 0L
 
-    /** Ordinary starts are rejected while busy; a restart may wait for the current attempt. */
+    /** Why [run] declined a start. Decided atomically with the gate state. */
+    enum class Rejection {
+        /** Another start owns the gate and will publish its own result. */
+        START_IN_PROGRESS,
+        /** A stop is running or completed while this start waited; nothing owns startup. */
+        STOPPING,
+    }
+
+    /**
+     * Ordinary starts are rejected while busy; a restart may wait for the current attempt.
+     * [onRejected] runs, outside the lock, with the reason when the start is declined.
+     */
     suspend fun run(
         waitForIdle: Boolean = false,
+        onRejected: (Rejection) -> Unit = {},
         block: suspend (Attempt) -> Unit,
     ): Boolean = coroutineScope {
         val job = coroutineContext.job
         val generation = synchronized(stateLock) {
-            if (pendingStops != 0) return@coroutineScope false
-            stopGeneration
-        }
+            if (pendingStops != 0) null else stopGeneration
+        } ?: return@coroutineScope reject(Rejection.STOPPING, onRejected)
         if (waitForIdle) operationMutex.lock()
-        synchronized(stateLock) {
+        val rejection = synchronized(stateLock) {
             // A restart queued before Stop must not reconnect after teardown.
             if (pendingStops != 0 || generation != stopGeneration) {
                 if (waitForIdle) operationMutex.unlock()
-                return@coroutineScope false
+                return@synchronized Rejection.STOPPING
             }
-            if (!waitForIdle && !operationMutex.tryLock()) return@coroutineScope false
+            if (!waitForIdle && !operationMutex.tryLock()) return@synchronized Rejection.START_IN_PROGRESS
             activeStart = job
+            null
         }
+        if (rejection != null) return@coroutineScope reject(rejection, onRejected)
         try {
             job.ensureActive()
             block(Attempt(job))
@@ -58,6 +71,11 @@ internal class VpnStartGate(
         }
     }
 
+    private fun reject(rejection: Rejection, onRejected: (Rejection) -> Unit): Boolean {
+        onRejected(rejection)
+        return false
+    }
+
     suspend fun stop(
         onStopping: () -> Unit = {},
         block: suspend () -> Unit,

android/app/src/test/kotlin/org/getlantern/lantern/service/VpnStartGateTest.kt

@@ -434,6 +434,68 @@ class VpnStartGateTest {
         assertTrue(gate.run {})
     }
 
+    @Test(timeout = 10_000)
+    fun duplicateRejectedByInFlightStartReportsStartInProgress() = runBlocking {
+        val gate = VpnStartGate()
+        val prepared = CompletableDeferred<Unit>()
+        val finish = CompletableDeferred<Unit>()
+        val rejections = mutableListOf<VpnStartGate.Rejection>()
+        val first = launch {
+            gate.run {
+                prepared.complete(Unit)
+                finish.await()
+            }
+        }
+        prepared.await()
+
+        assertFalse(gate.run(onRejected = { rejections.add(it) }) { error("duplicate must not run") })
+        assertEquals(listOf(VpnStartGate.Rejection.START_IN_PROGRESS), rejections)
+
+        finish.complete(Unit)
+        first.join()
+        assertTrue(gate.run(onRejected = { error("accepted start must not report a rejection") }) {})
+    }
+
+    @Test(timeout = 10_000)
+    fun startsRejectedByStopReportStopping() = runBlocking {
+        val gate = VpnStartGate()
+        val entered = CompletableDeferred<Unit>()
+        val finish = CompletableDeferred<Unit>()
+        val rejections = mutableListOf<VpnStartGate.Rejection>()
+        val stop = launch {
+            gate.stop {
+                entered.complete(Unit)
+                finish.await()
+            }
+        }
+        entered.await()
+
+        // Rejected up front: a stop is already pending.
+        assertFalse(gate.run(onRejected = { rejections.add(it) }) { error("stop is running") })
+        finish.complete(Unit)
+        stop.join()
+
+        // Rejected after waiting: a restart queued before Stop finished must not reconnect.
+        val prepared = CompletableDeferred<Unit>()
+        val busy = launch {
+            gate.run {
+                prepared.complete(Unit)
+                CompletableDeferred<Unit>().await()
+            }
+        }
+        prepared.await()
+        val restart = async(start = CoroutineStart.UNDISPATCHED) {
+            gate.run(waitForIdle = true, onRejected = { rejections.add(it) }) { error("restart must not undo Stop") }
+        }
+        val secondStop = launch { gate.stop {} }
+        busy.join()
+        assertFalse(restart.await())
+        secondStop.join()
+
+        assertEquals(listOf(VpnStartGate.Rejection.STOPPING, VpnStartGate.Rejection.STOPPING), rejections)
+        assertTrue(gate.run {})
+    }
+
     @Test(timeout = 10_000)
     fun cancellingStopDoesNotAbandonTeardown() = runBlocking {
         val gate = VpnStartGate()