mirror of
https://github.com/permissionlesstech/bitchat-android.git
synced 2026-08-29 07:16:08 +00:00
fix(wifi-aware): count hotspot radio holds and never drop a restart request
Two fixes to how Wi-Fi Aware yields the radio to the Wi-Fi Direct hotspot. A restart request could be swallowed: restartIfStillEnabled() coalesces on an in-flight flag, so a request arriving while an earlier loop was still burning attempts against the hotspot hold lost the CAS and was dropped. The loop then exhausted its attempts without ever seeing the cleared hold, leaving the mesh down despite the user's setting. Requests are now recorded before coalescing and re-checked after each pass. The hold itself was a single flag, so overlapping share sessions could release each other's claim. It is now a counted, once-releasable HotspotLease: Aware restarts only when no session still needs the radio, and a duplicate or late release is a no-op. Publication of a started service is checked against the hold inside the same lock stop() takes, so a start racing a new hold cannot resurrect NAN while the hotspot owns the radio. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
97d6e8a479
commit
3562ad10d5
@ -11,7 +11,9 @@ import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlinx.coroutines.launch
|
||||
import androidx.annotation.VisibleForTesting
|
||||
import java.util.concurrent.atomic.AtomicBoolean
|
||||
import java.util.concurrent.atomic.AtomicInteger
|
||||
|
||||
/**
|
||||
* WifiAwareController manages lifecycle and debug surfacing for the WifiAwareMeshService.
|
||||
@ -27,17 +29,29 @@ object WifiAwareController {
|
||||
private val lifecycleLock = Any()
|
||||
private var starting = false
|
||||
private val restartInFlight = AtomicBoolean(false)
|
||||
private val restartRequested = AtomicBoolean(false)
|
||||
private var awareReceiverRegistered = false
|
||||
private var lastBlockedReason: String? = null
|
||||
|
||||
/**
|
||||
* Set while a Wi-Fi Direct hotspot is hosting. Wi-Fi Aware (NAN) and Wi-Fi Direct
|
||||
* (P2P) cannot hold interfaces at the same time on common chipsets — the HAL fails
|
||||
* to create the P2P iface and every createGroup is answered with BUSY. The hold
|
||||
* also blocks [startIfPossible], so a resume or mesh-service restart cannot bring
|
||||
* Aware back while the hotspot is up.
|
||||
* How many hotspot sessions currently need the radio.
|
||||
*
|
||||
* Wi-Fi Aware (NAN) and Wi-Fi Direct (P2P) cannot hold interfaces at the same time on
|
||||
* common chipsets — the HAL fails to create the P2P iface and every createGroup is
|
||||
* answered with BUSY. A non-zero count also blocks [startIfPossible], so a resume or
|
||||
* mesh-service restart cannot bring Aware back while a hotspot is up.
|
||||
*
|
||||
* A count rather than a flag because teardown is asynchronous and sessions can
|
||||
* overlap. Callers receive a once-releasable [HotspotLease], so one session cannot
|
||||
* accidentally consume another session's hold.
|
||||
*/
|
||||
private val hotspotHold = AtomicBoolean(false)
|
||||
private val hotspotHolds = AtomicInteger(0)
|
||||
|
||||
/** True while any hotspot session still needs the radio. */
|
||||
@VisibleForTesting
|
||||
internal fun isHeldForHotspot(): Boolean = hotspotHolds.get() > 0
|
||||
|
||||
private fun heldForHotspot(): Boolean = isHeldForHotspot()
|
||||
|
||||
private val scope = CoroutineScope(Dispatchers.IO + SupervisorJob())
|
||||
|
||||
@ -97,26 +111,65 @@ object WifiAwareController {
|
||||
}
|
||||
|
||||
/**
|
||||
* Releases the Wi-Fi radio so a Wi-Fi Direct hotspot can create its P2P interface,
|
||||
* and prevents Aware restarting until [releaseHotspotHold] is called.
|
||||
* A session-scoped claim on the Wi-Fi radio.
|
||||
*
|
||||
* Closing the same lease twice is a no-op; raw decrements are intentionally not
|
||||
* exposed because lifecycle retries and duplicate teardown calls must not release a
|
||||
* different hotspot session's hold.
|
||||
*/
|
||||
fun holdForHotspot() {
|
||||
if (!hotspotHold.compareAndSet(false, true)) return
|
||||
Log.i(TAG, "Holding Wi-Fi Aware down so the hotspot can use the radio")
|
||||
stop()
|
||||
class HotspotLease internal constructor(
|
||||
private val releaseAction: () -> Unit
|
||||
) : AutoCloseable {
|
||||
private val released = AtomicBoolean(false)
|
||||
|
||||
override fun close() {
|
||||
if (released.compareAndSet(false, true)) {
|
||||
releaseAction()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Drops the hold and restores Aware if the user still has it enabled. */
|
||||
fun releaseHotspotHold() {
|
||||
if (!hotspotHold.compareAndSet(true, false)) return
|
||||
/**
|
||||
* Releases the Wi-Fi radio so a Wi-Fi Direct hotspot can create its P2P interface,
|
||||
* and prevents Aware restarting until the returned lease is closed.
|
||||
*
|
||||
* Counted because a second share can start while the first is still cleaning up.
|
||||
*/
|
||||
fun acquireHotspotLease(): HotspotLease {
|
||||
if (hotspotHolds.getAndIncrement() == 0) {
|
||||
Log.i(TAG, "Holding Wi-Fi Aware down so the hotspot can use the radio")
|
||||
stop()
|
||||
}
|
||||
return HotspotLease(::releaseHotspotLease)
|
||||
}
|
||||
|
||||
private fun releaseHotspotLease() {
|
||||
var remaining: Int
|
||||
while (true) {
|
||||
val current = hotspotHolds.get()
|
||||
if (current == 0) return
|
||||
remaining = current - 1
|
||||
if (hotspotHolds.compareAndSet(current, remaining)) break
|
||||
}
|
||||
|
||||
if (remaining != 0) {
|
||||
Log.d(TAG, "Hotspot finished, but $remaining other hold(s) remain")
|
||||
return
|
||||
}
|
||||
|
||||
Log.i(TAG, "Hotspot finished; restoring Wi-Fi Aware if enabled")
|
||||
restartIfStillEnabled()
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
internal fun resetHotspotLeasesForTest() {
|
||||
hotspotHolds.set(0)
|
||||
}
|
||||
|
||||
fun startIfPossible() {
|
||||
val reusableService = synchronized(lifecycleLock) {
|
||||
if (!_enabled.value) return
|
||||
if (hotspotHold.get()) {
|
||||
if (heldForHotspot()) {
|
||||
Log.d(TAG, "Not starting Wi-Fi Aware: held down for the hotspot")
|
||||
return
|
||||
}
|
||||
@ -179,7 +232,7 @@ object WifiAwareController {
|
||||
return
|
||||
}
|
||||
}
|
||||
if (!_enabled.value || hotspotHold.get()) {
|
||||
if (!_enabled.value || heldForHotspot()) {
|
||||
synchronized(lifecycleLock) { starting = false }
|
||||
return
|
||||
}
|
||||
@ -191,13 +244,13 @@ object WifiAwareController {
|
||||
startedService.startServices()
|
||||
|
||||
// Test the hold inside the same lock that publishes the service, and that
|
||||
// stop() takes. Testing it outside leaves a window where holdForHotspot()
|
||||
// stop() takes. Testing it outside leaves a window where acquiring a lease
|
||||
// sets the flag and stop() finds nothing published yet, and this block then
|
||||
// publishes anyway — resurrecting NAN while the hotspot owns the radio.
|
||||
// Ordering holds because holdForHotspot() sets the flag before calling
|
||||
// Ordering holds because acquireHotspotLease() increments before calling
|
||||
// stop(): either we see the flag here, or stop() sees our published service.
|
||||
val published = synchronized(lifecycleLock) {
|
||||
val canPublish = !hotspotHold.get() && startedService.isRunning()
|
||||
val canPublish = !heldForHotspot() && startedService.isRunning()
|
||||
if (canPublish) {
|
||||
service = startedService
|
||||
_running.value = true
|
||||
@ -214,7 +267,7 @@ object WifiAwareController {
|
||||
try { com.bitchat.android.ui.debug.DebugSettingsManager.getInstance().addDebugMessage(com.bitchat.android.ui.debug.DebugMessage.SystemMessage("Wi‑Fi Aware started")) } catch (_: Exception) {}
|
||||
} else {
|
||||
// stopServices() can block, so keep it out of the lock.
|
||||
val heldForHotspot = hotspotHold.get()
|
||||
val heldForHotspot = heldForHotspot()
|
||||
if (heldForHotspot || reusableService == null) {
|
||||
try { startedService.stopServices() } catch (_: Exception) { }
|
||||
}
|
||||
@ -264,6 +317,13 @@ object WifiAwareController {
|
||||
* startServices() defers and we must try again rather than give up.
|
||||
*/
|
||||
internal fun restartIfStillEnabled(delayMs: Long = 0L) {
|
||||
// Record the request before coalescing. A request that arrives while a loop is
|
||||
// already running — the final lease closing during a loop that has been
|
||||
// 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.
|
||||
restartRequested.set(true)
|
||||
|
||||
if (!restartInFlight.compareAndSet(false, true)) {
|
||||
Log.d(TAG, "Restart already in flight; coalescing request")
|
||||
return
|
||||
@ -271,18 +331,30 @@ object WifiAwareController {
|
||||
scope.launch {
|
||||
try {
|
||||
if (delayMs > 0L) delay(delayMs)
|
||||
var attempt = 0
|
||||
while (_enabled.value && !_running.value && attempt < MAX_RESTART_ATTEMPTS) {
|
||||
val ctx = appContext
|
||||
if (ctx != null && !refreshSupportStatus(ctx).supported) break
|
||||
startIfPossible()
|
||||
if (_running.value) break
|
||||
attempt++
|
||||
delay(RESTART_RETRY_DELAY_MS)
|
||||
}
|
||||
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) {
|
||||
val ctx = appContext
|
||||
if (ctx != null && !refreshSupportStatus(ctx).supported) break
|
||||
startIfPossible()
|
||||
if (_running.value) break
|
||||
attempt++
|
||||
delay(RESTART_RETRY_DELAY_MS)
|
||||
}
|
||||
} while (restartRequested.get() && _enabled.value && !_running.value)
|
||||
} finally {
|
||||
restartInFlight.set(false)
|
||||
}
|
||||
|
||||
// A request landing between the loop exiting and the flag clearing finds
|
||||
// restartInFlight still set and returns; pick it up here. Terminates because
|
||||
// each pass consumes the flag before doing any work.
|
||||
if (restartRequested.get() && _enabled.value && !_running.value) {
|
||||
restartIfStillEnabled()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -0,0 +1,73 @@
|
||||
package com.bitchat.android.wifiaware
|
||||
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
|
||||
/**
|
||||
* The hold keeps Wi-Fi Aware off the radio while a hotspot uses it. Getting the
|
||||
* bookkeeping wrong is not cosmetic: release it early and the P2P group cannot form,
|
||||
* fail to release it and the mesh stays down.
|
||||
*
|
||||
* Teardown is asynchronous, so sessions overlap and releases arrive out of order. These
|
||||
* cover the counting rules that make that safe.
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class WifiAwareHotspotHoldTest {
|
||||
|
||||
@Before
|
||||
fun clearAnyLeftoverHolds() {
|
||||
WifiAwareController.resetHotspotLeasesForTest()
|
||||
}
|
||||
|
||||
@After
|
||||
fun clearTestHolds() {
|
||||
WifiAwareController.resetHotspotLeasesForTest()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a single session holds and then releases`() {
|
||||
val lease = WifiAwareController.acquireHotspotLease()
|
||||
assertTrue(WifiAwareController.isHeldForHotspot())
|
||||
|
||||
lease.close()
|
||||
assertFalse(WifiAwareController.isHeldForHotspot())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a second session starting before the first finishes keeps the radio held`() {
|
||||
val first = WifiAwareController.acquireHotspotLease()
|
||||
val second = WifiAwareController.acquireHotspotLease()
|
||||
|
||||
// The first session's asynchronous teardown completes.
|
||||
first.close()
|
||||
|
||||
assertTrue(
|
||||
"the second session still needs the radio",
|
||||
WifiAwareController.isHeldForHotspot()
|
||||
)
|
||||
|
||||
second.close()
|
||||
assertFalse(WifiAwareController.isHeldForHotspot())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `closing one lease twice cannot release another session`() {
|
||||
val first = WifiAwareController.acquireHotspotLease()
|
||||
val second = WifiAwareController.acquireHotspotLease()
|
||||
|
||||
first.close()
|
||||
first.close()
|
||||
assertTrue(WifiAwareController.isHeldForHotspot())
|
||||
|
||||
second.close()
|
||||
assertFalse(
|
||||
"only the matching session can release its radio claim",
|
||||
WifiAwareController.isHeldForHotspot()
|
||||
)
|
||||
}
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user