From 7faa646f23fe1dcc54bcb3fdae1630ba56882b38 Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Sun, 8 Nov 2026 02:23:09 -0500 Subject: [PATCH 1/2] fix(wifi-aware): restart after an unexpected stop handleUnexpectedStop never told the controller, so it kept reporting running and every restart request did nothing. Aware stayed down until an outside start such as an Activity resume. Clear the controller's running state while keeping the instance for reuse, and fix what same-instance restart exposed: orphaning on a deferred start, republishing after stop(), recovery cleanup reading the coroutine's isActive, duplicate background loops, and unbounded attach retries. --- .../android/mesh/MeshConnectionTracker.kt | 6 +- .../android/wifi-aware/WifiAwareController.kt | 30 ++- .../wifi-aware/WifiAwareMeshService.kt | 40 +++- .../WifiAwareUnexpectedStopRestartTest.kt | 185 ++++++++++++++++++ 4 files changed, 248 insertions(+), 13 deletions(-) create mode 100644 app/src/test/kotlin/com/bitchat/android/wifiaware/WifiAwareUnexpectedStopRestartTest.kt diff --git a/app/src/main/java/com/bitchat/android/mesh/MeshConnectionTracker.kt b/app/src/main/java/com/bitchat/android/mesh/MeshConnectionTracker.kt index 0dd6a552e..e2d52765c 100644 --- a/app/src/main/java/com/bitchat/android/mesh/MeshConnectionTracker.kt +++ b/app/src/main/java/com/bitchat/android/mesh/MeshConnectionTracker.kt @@ -3,6 +3,7 @@ package com.bitchat.android.mesh import android.util.Log import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.launch import java.util.concurrent.ConcurrentHashMap @@ -43,6 +44,7 @@ abstract class MeshConnectionTracker( protected val pendingConnections = ConcurrentHashMap() private var isActive = false + private var cleanupJob: Job? = null /** * Start the tracker and its cleanup loop @@ -119,7 +121,9 @@ abstract class MeshConnectionTracker( abstract fun getConnectionCount(): Int private fun startPeriodicCleanup() { - scope.launch { + // A stop/start can complete while the previous loop is in delay(); one loop must remain. + cleanupJob?.cancel() + cleanupJob = scope.launch { while (isActive) { try { delay(CLEANUP_INTERVAL) diff --git a/app/src/main/java/com/bitchat/android/wifi-aware/WifiAwareController.kt b/app/src/main/java/com/bitchat/android/wifi-aware/WifiAwareController.kt index d5a694f15..dd6cb3d73 100644 --- a/app/src/main/java/com/bitchat/android/wifi-aware/WifiAwareController.kt +++ b/app/src/main/java/com/bitchat/android/wifi-aware/WifiAwareController.kt @@ -249,13 +249,22 @@ object WifiAwareController { // publishes anyway — resurrecting NAN while the hotspot owns the radio. // Ordering holds because acquireHotspotLease() increments before calling // stop(): either we see the flag here, or stop() sees our published service. + // + // stop() withdraws an instance by clearing `service`. A reused instance it took + // during this attempt is being torn down and must not be published again. + var retainedForRetry = false val published = synchronized(lifecycleLock) { - val canPublish = !heldForHotspot() && startedService.isRunning() + val withdrawn = reusableService != null && service !== reusableService + val canPublish = !heldForHotspot() && !withdrawn && startedService.isRunning() if (canPublish) { service = startedService _running.value = true } else { - if (service === startedService) service = null + if (heldForHotspot() && service === startedService) service = null + // A reused instance can defer its start while recovery cleanup finishes. + // Keep it for the next attempt: it was never shut down, so replacing it + // would orphan it next to a second instance. + retainedForRetry = service === startedService _running.value = false } canPublish @@ -268,7 +277,7 @@ object WifiAwareController { } else { // stopServices() can block, so keep it out of the lock. val heldForHotspot = heldForHotspot() - if (heldForHotspot || reusableService == null) { + if (!retainedForRetry) { try { startedService.stopServices() } catch (_: Exception) { } } if (heldForHotspot) { @@ -310,6 +319,21 @@ object WifiAwareController { } } + /** + * Records that the published service lost its sessions but was not shut down. + * + * Unlike [onServiceStopped] this keeps the reference: the instance still owns its scope + * and core, and [startIfPossible] resumes it. Clearing [running] is what lets + * [restartIfStillEnabled] act. + */ + internal fun onServiceInterrupted(interruptedService: WifiAwareMeshService) { + synchronized(lifecycleLock) { + if (service !== interruptedService) return + _running.value = false + clearDebugSnapshot() + } + } + /** * Schedules a restart of the Wi-Fi Aware transport. Concurrent requests are coalesced into * a single in-flight loop that retries with backoff. This is important because a single fixed diff --git a/app/src/main/java/com/bitchat/android/wifi-aware/WifiAwareMeshService.kt b/app/src/main/java/com/bitchat/android/wifi-aware/WifiAwareMeshService.kt index 93aabb17b..1dd8244dc 100644 --- a/app/src/main/java/com/bitchat/android/wifi-aware/WifiAwareMeshService.kt +++ b/app/src/main/java/com/bitchat/android/wifi-aware/WifiAwareMeshService.kt @@ -77,6 +77,19 @@ class WifiAwareMeshService(private val context: Context) : MeshService, Transpor private const val CLIENT_SOCKET_ATTEMPTS = 3 private const val CLIENT_ROLE_REVERSAL_FAILURES = 3 private const val ROLE_REVERSAL_PREFIX = "ROLE_SERVER:" + private const val RECOVERY_RESTART_DELAY_MS = 500L + private const val ATTACH_RETRY_BASE_MS = 3_000L + private const val ATTACH_RETRY_MAX_MS = 60_000L + + /** + * Delay before re-attaching after [consecutiveFailures] failed attaches in a row. + * + * A start counts as running until its attach is answered, so the controller's attempt + * limit never trips on repeated attach failures. This keeps them off the radio instead. + */ + internal fun attachRetryDelayMs(consecutiveFailures: Int): Long = + (ATTACH_RETRY_BASE_MS shl (consecutiveFailures - 1).coerceIn(0, 5)) + .coerceAtMost(ATTACH_RETRY_MAX_MS) } // Core crypto/services @@ -105,6 +118,7 @@ class WifiAwareMeshService(private val context: Context) : MeshService, Transpor @Volatile private var isActive = false @Volatile private var recoveryInProgress = false private val sessionGeneration = AtomicInteger(0) + private val consecutiveAttachFailures = AtomicInteger(0) // Delegate override var delegate: WifiAwareMeshDelegate? = null @@ -363,6 +377,7 @@ class WifiAwareMeshService(private val context: Context) : MeshService, Transpor return } wifiAwareSession = session + consecutiveAttachFailures.set(0) Log.i(TAG, "Wi-Fi Aware attached; starting publish & subscribe") // PUBLISH (server role) @@ -493,9 +508,10 @@ class WifiAwareMeshService(private val context: Context) : MeshService, Transpor override fun onAttachFailed() { if (!isCurrentSession(generation)) return Log.e(TAG, "Wi-Fi Aware attach failed") - handleUnexpectedStop(generation) + val retryDelayMs = attachRetryDelayMs(consecutiveAttachFailures.incrementAndGet()) + handleUnexpectedStop(generation, retryDelayMs) if (com.bitchat.android.wifiaware.WifiAwareController.enabled.value) { - com.bitchat.android.wifiaware.WifiAwareController.restartIfStillEnabled(3000) + com.bitchat.android.wifiaware.WifiAwareController.restartIfStillEnabled(retryDelayMs) } } @@ -516,7 +532,7 @@ class WifiAwareMeshService(private val context: Context) : MeshService, Transpor meshCore.startCore() com.bitchat.android.service.MeshServiceHolder.startSharedGossip("WIFI") - startPeriodicConnectionMaintenance() + startPeriodicConnectionMaintenance(generation) connectionTracker.start() } @@ -569,13 +585,16 @@ class WifiAwareMeshService(private val context: Context) : MeshService, Transpor return generation == sessionGeneration.get() && isActive } - private fun handleUnexpectedStop(generation: Int) { + private fun handleUnexpectedStop(generation: Int, restartDelayMs: Long = RECOVERY_RESTART_DELAY_MS) { if (generation != sessionGeneration.get()) return if (!isActive) { return } recoveryInProgress = true isActive = false + // Must follow isActive = false so a concurrent publish in the controller is either + // refused or overwritten here. Must precede the restart requests made by callers. + com.bitchat.android.wifiaware.WifiAwareController.onServiceInterrupted(this) TransportBridgeService.unregister("WIFI") com.bitchat.android.service.MeshServiceHolder.stopSharedGossip("WIFI") try { com.bitchat.android.services.AppStateStore.clearTransportPeers("WIFI") } catch (_: Exception) { } @@ -590,7 +609,8 @@ class WifiAwareMeshService(private val context: Context) : MeshService, Transpor try { oldPublishSession?.close() } catch (_: Exception) { } try { oldSubscribeSession?.close() } catch (_: Exception) { } try { oldWifiAwareSession?.close() } catch (_: Exception) { } - if (generation == sessionGeneration.get() && !isActive) { + // Qualified: a bare isActive here is the coroutine's, which is always true. + if (generation == sessionGeneration.get() && !this@WifiAwareMeshService.isActive) { if (publishSession === oldPublishSession) publishSession = null if (subscribeSession === oldSubscribeSession) subscribeSession = null if (wifiAwareSession === oldWifiAwareSession) wifiAwareSession = null @@ -605,7 +625,7 @@ class WifiAwareMeshService(private val context: Context) : MeshService, Transpor // Recovery cleanup is done; nudge a restart now that startServices() will no // longer be deferred by recoveryInProgress. The controller coalesces requests. if (com.bitchat.android.wifiaware.WifiAwareController.enabled.value) { - com.bitchat.android.wifiaware.WifiAwareController.restartIfStillEnabled(500) + com.bitchat.android.wifiaware.WifiAwareController.restartIfStillEnabled(restartDelayMs) } } } @@ -645,13 +665,15 @@ class WifiAwareMeshService(private val context: Context) : MeshService, Transpor /** * Periodic active maintenance: retries connections to discovered but unconnected peers. */ - private fun startPeriodicConnectionMaintenance() { + private fun startPeriodicConnectionMaintenance(generation: Int) { serviceScope.launch { - while (isActive) { + // Scoped to the session: a restart can finish while this loop is in delay(), and + // an isActive check alone would leave it running next to the new session's loop. + while (isCurrentSession(generation)) { try { val schedule = powerManager.profile.value.wifiAware delay(schedule.connectionMaintenanceMs) - if (!isActive) break + if (!isCurrentSession(generation)) break val now = System.currentTimeMillis() diff --git a/app/src/test/kotlin/com/bitchat/android/wifiaware/WifiAwareUnexpectedStopRestartTest.kt b/app/src/test/kotlin/com/bitchat/android/wifiaware/WifiAwareUnexpectedStopRestartTest.kt new file mode 100644 index 000000000..f327bef2e --- /dev/null +++ b/app/src/test/kotlin/com/bitchat/android/wifiaware/WifiAwareUnexpectedStopRestartTest.kt @@ -0,0 +1,185 @@ +package com.bitchat.android.wifiaware + +import android.app.Application +import android.content.Context +import android.content.ContextWrapper +import android.content.pm.PackageManager +import android.location.LocationManager +import android.net.wifi.aware.AttachCallback +import android.net.wifi.aware.WifiAwareManager +import android.os.Handler +import androidx.test.core.app.ApplicationProvider +import com.bitchat.android.crypto.EncryptionService +import com.bitchat.android.mesh.BluetoothMeshService +import com.bitchat.android.mesh.FragmentingPacketSender +import com.bitchat.android.mesh.MeshCore +import com.bitchat.android.service.MeshServiceHolder +import com.bitchat.android.ui.NotificationManager +import java.util.concurrent.atomic.AtomicBoolean +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.ArgumentCaptor +import org.mockito.MockedConstruction +import org.mockito.Mockito +import org.mockito.kotlin.any +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config + +/** + * The radio can end an Aware session at any time, and the service also ends its own + * sessions to refresh discovery. Each of those stops has to come back on its own: the + * user's setting says Aware is on, and no Activity resume may be coming. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [33]) +class WifiAwareUnexpectedStopRestartTest { + + private val awareManager: WifiAwareManager = mock() + private lateinit var context: Context + private val constructionStubs = mutableListOf>() + + @Before + fun setUp() { + // Robolectric has no AndroidKeyStore, and these collaborators are not under test. + constructionStubs += Mockito.mockConstruction(EncryptionService::class.java) { mock, _ -> + whenever(mock.getIdentityFingerprint()).thenReturn(SYNTHETIC_FINGERPRINT) + } + constructionStubs += Mockito.mockConstruction(BluetoothMeshService::class.java) + constructionStubs += Mockito.mockConstruction(NotificationManager::class.java) + constructionStubs += Mockito.mockConstruction(MeshCore::class.java) + constructionStubs += Mockito.mockConstruction(FragmentingPacketSender::class.java) + val app = ApplicationProvider.getApplicationContext() + shadowOf(app.packageManager).setSystemFeature(PackageManager.FEATURE_WIFI_AWARE, true) + shadowOf(app.getSystemService(LocationManager::class.java)).setLocationEnabled(true) + shadowOf(app).grantPermissions(android.Manifest.permission.NEARBY_WIFI_DEVICES) + whenever(awareManager.isAvailable).thenReturn(true) + context = object : ContextWrapper(app) { + override fun getApplicationContext(): Context = this + override fun getSystemService(name: String): Any? = + if (name == Context.WIFI_AWARE_SERVICE) awareManager else super.getSystemService(name) + } + WifiAwareController.resetHotspotLeasesForTest() + } + + @After + fun tearDown() { + WifiAwareController.setEnabled(false) + WifiAwareController.resetHotspotLeasesForTest() + MeshServiceHolder.clear() + constructionStubs.forEach { it.close() } + } + + @Test + fun `an unexpected session end re-attaches the same service without an outside start`() { + WifiAwareController.initialize(context, enabledByDefault = true) + val service = WifiAwareController.getService()!! + assertTrue(WifiAwareController.running.value) + + val callback = ArgumentCaptor.forClass(AttachCallback::class.java) + Mockito.verify(awareManager).attach(callback.capture(), any()) + + callback.value.onAwareSessionTerminated() + + assertFalse( + "the controller must not report a stopped transport as running", + WifiAwareController.running.value + ) + runBlocking { + withTimeout(RESTART_TIMEOUT_MS) { WifiAwareController.running.first { it } } + } + Mockito.verify(awareManager, Mockito.times(2)).attach(any(), any()) + assertSame( + "recovery reuses the instance rather than orphaning it", + service, + WifiAwareController.getService() + ) + } + + @Test + fun `a restart deferred by recovery cleanup keeps the instance for the next attempt`() { + withStubbedService { constructed, _ -> + val service = constructed.constructed().single() + WifiAwareController.onServiceInterrupted(service) + + // The stub stays not-running through startServices(), as a deferred start does. + WifiAwareController.startIfPossible() + + assertFalse(WifiAwareController.running.value) + assertSame(service, WifiAwareController.getService()) + assertEquals(1, constructed.constructed().size) + verify(service, never()).stopServices() + } + } + + @Test + fun `a stop racing a reused start is not undone by the start`() { + withStubbedService { constructed, isRunning -> + val service = constructed.constructed().single() + WifiAwareController.onServiceInterrupted(service) + whenever(service.startServices()).thenAnswer { + WifiAwareController.stop() + isRunning.set(true) + } + + WifiAwareController.startIfPossible() + + assertFalse(WifiAwareController.running.value) + assertNull(WifiAwareController.getService()) + } + } + + @Test + fun `an interruption from a replaced instance leaves the current one running`() { + withStubbedService(initiallyRunning = true) { constructed, _ -> + val current = constructed.constructed().single() + + WifiAwareController.onServiceInterrupted(mock()) + + assertTrue(WifiAwareController.running.value) + assertSame(current, WifiAwareController.getService()) + } + } + + @Test + fun `repeated attach failures back off to a capped retry interval`() { + val delays = (1..8).map(WifiAwareMeshService::attachRetryDelayMs) + + assertEquals(listOf(3_000L, 6_000L, 12_000L, 24_000L, 48_000L, 60_000L, 60_000L, 60_000L), delays) + } + + private fun withStubbedService( + initiallyRunning: Boolean = false, + block: (MockedConstruction, AtomicBoolean) -> Unit + ) { + // Starts out running so the controller publishes it, as a healthy first start does. + val isRunning = AtomicBoolean(true) + Mockito.mockConstruction(WifiAwareMeshService::class.java) { mock, _ -> + whenever(mock.isRunning()).thenAnswer { isRunning.get() } + }.use { constructed -> + WifiAwareController.initialize(context, enabledByDefault = true) + isRunning.set(initiallyRunning) + block(constructed, isRunning) + } + } + + private companion object { + // Longer than the service's restart delay plus one controller retry. + const val RESTART_TIMEOUT_MS = 20_000L + const val SYNTHETIC_FINGERPRINT = "00112233445566778899aabbccddeeff" + } +} From c49bae6e742b9233b9a45991fa42012dca9ee7e6 Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Sun, 8 Nov 2026 02:37:21 -0500 Subject: [PATCH 2/2] fix(wifi-aware): keep the restart cooldown when requests coalesce A request arriving while a restart loop was in flight recorded only a Boolean, so its delay was dropped and the loop retried after the fixed 2s. Record a not-before deadline and wait for it before every attempt. --- .../android/wifi-aware/WifiAwareController.kt | 26 +++++++++++++- .../WifiAwareUnexpectedStopRestartTest.kt | 34 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/bitchat/android/wifi-aware/WifiAwareController.kt b/app/src/main/java/com/bitchat/android/wifi-aware/WifiAwareController.kt index dd6cb3d73..f59fb2061 100644 --- a/app/src/main/java/com/bitchat/android/wifi-aware/WifiAwareController.kt +++ b/app/src/main/java/com/bitchat/android/wifi-aware/WifiAwareController.kt @@ -14,6 +14,7 @@ import kotlinx.coroutines.launch import androidx.annotation.VisibleForTesting import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicInteger +import java.util.concurrent.atomic.AtomicLong /** * WifiAwareController manages lifecycle and debug surfacing for the WifiAwareMeshService. @@ -30,6 +31,9 @@ object WifiAwareController { private var starting = false private val restartInFlight = AtomicBoolean(false) private val restartRequested = AtomicBoolean(false) + // Monotonic. A deadline rather than a delay, so a request coalesced into a running loop + // still holds that loop back; the latest deadline requested wins. + private val restartNotBeforeMs = AtomicLong(Long.MIN_VALUE) private var awareReceiverRegistered = false private var lastBlockedReason: String? = null @@ -346,6 +350,9 @@ object WifiAwareController { // burning attempts against the hold — would otherwise be dropped, and the old // loop would exhaust MAX_RESTART_ATTEMPTS without ever seeing the cleared hold, // leaving Aware down despite the user's setting. + restartNotBeforeMs.accumulateAndGet(monotonicNowMs() + delayMs) { current, requested -> + maxOf(current, requested) + } restartRequested.set(true) if (!restartInFlight.compareAndSet(false, true)) { @@ -354,13 +361,16 @@ object WifiAwareController { } scope.launch { try { - if (delayMs > 0L) delay(delayMs) + awaitRestartDeadline() do { // Consume the request before working, so anything arriving during // the attempts below schedules another pass. restartRequested.set(false) var attempt = 0 while (_enabled.value && !_running.value && attempt < MAX_RESTART_ATTEMPTS) { + // A failure answered during the previous attempt may have moved this. + awaitRestartDeadline() + if (!_enabled.value || _running.value) break val ctx = appContext if (ctx != null && !refreshSupportStatus(ctx).supported) break startIfPossible() @@ -382,6 +392,20 @@ object WifiAwareController { } } + private fun monotonicNowMs(): Long = System.nanoTime() / 1_000_000L + + /** Re-reads the deadline after each wait, since a coalesced request can extend it. */ + private suspend fun awaitRestartDeadline() { + while (true) { + val remainingMs = restartNotBeforeMs.get().let { deadline -> + val now = monotonicNowMs() + if (deadline <= now) 0L else deadline - now + } + if (remainingMs == 0L) return + delay(remainingMs) + } + } + /** * Listens for system Wi-Fi Aware availability changes. Aware can flip off/on at runtime * (Wi-Fi toggling, hotspot/SoftAP, location changes); without this we would only recover on diff --git a/app/src/test/kotlin/com/bitchat/android/wifiaware/WifiAwareUnexpectedStopRestartTest.kt b/app/src/test/kotlin/com/bitchat/android/wifiaware/WifiAwareUnexpectedStopRestartTest.kt index f327bef2e..68afe9fcc 100644 --- a/app/src/test/kotlin/com/bitchat/android/wifiaware/WifiAwareUnexpectedStopRestartTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/wifiaware/WifiAwareUnexpectedStopRestartTest.kt @@ -15,6 +15,8 @@ import com.bitchat.android.mesh.FragmentingPacketSender import com.bitchat.android.mesh.MeshCore import com.bitchat.android.service.MeshServiceHolder import com.bitchat.android.ui.NotificationManager +import java.util.concurrent.CopyOnWriteArrayList +import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicBoolean import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking @@ -155,6 +157,36 @@ class WifiAwareUnexpectedStopRestartTest { } } + @Test + fun `a cooldown requested during a restart attempt delays the next attempt`() { + withStubbedService { constructed, isRunning -> + val service = constructed.constructed().single() + WifiAwareController.onServiceInterrupted(service) + val attemptTimesNs = CopyOnWriteArrayList() + whenever(service.startServices()).thenAnswer { + attemptTimesNs += System.nanoTime() + if (attemptTimesNs.size == 1) { + // An attach failure answered before the attempt finishes publishing. + WifiAwareController.restartIfStillEnabled(COALESCED_COOLDOWN_MS) + } else { + // Ends the loop so it cannot run into the next test. + isRunning.set(true) + } + } + + WifiAwareController.restartIfStillEnabled() + runBlocking { + withTimeout(RESTART_TIMEOUT_MS) { WifiAwareController.running.first { it } } + } + + val gapMs = TimeUnit.NANOSECONDS.toMillis(attemptTimesNs[1] - attemptTimesNs[0]) + assertTrue( + "next attempt came after ${gapMs}ms, inside the ${COALESCED_COOLDOWN_MS}ms cooldown", + gapMs >= COALESCED_COOLDOWN_MS + ) + } + } + @Test fun `repeated attach failures back off to a capped retry interval`() { val delays = (1..8).map(WifiAwareMeshService::attachRetryDelayMs) @@ -180,6 +212,8 @@ class WifiAwareUnexpectedStopRestartTest { private companion object { // Longer than the service's restart delay plus one controller retry. const val RESTART_TIMEOUT_MS = 20_000L + // Longer than the controller's fixed retry delay, so ignoring it is detectable. + const val COALESCED_COOLDOWN_MS = 4_500L const val SYNTHETIC_FINGERPRINT = "00112233445566778899aabbccddeeff" } }