diff --git a/app/src/main/java/com/bitchat/android/hotspot/HotspotActivity.kt b/app/src/main/java/com/bitchat/android/hotspot/HotspotActivity.kt index bf9745d9..35d0f4ec 100644 --- a/app/src/main/java/com/bitchat/android/hotspot/HotspotActivity.kt +++ b/app/src/main/java/com/bitchat/android/hotspot/HotspotActivity.kt @@ -28,12 +28,14 @@ import androidx.compose.ui.graphics.asImageBitmap import androidx.compose.ui.platform.LocalClipboardManager import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalDensity +import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.AnnotatedString import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import androidx.lifecycle.compose.collectAsStateWithLifecycle +import com.bitchat.android.R import com.bitchat.android.ui.theme.BitchatFontFamily import com.bitchat.android.ui.theme.BitchatTheme import com.bitchat.android.util.UniversalApkManager @@ -137,6 +139,12 @@ fun HotspotScreen( is HotspotViewModel.HotspotState.Starting -> { LoadingScreen() } + is HotspotViewModel.HotspotState.ConfirmDisconnect -> { + ExistingGroupConfirmation( + onConfirm = viewModel::confirmDisconnectAndStart, + onCancel = viewModel::cancelDisconnect + ) + } is HotspotViewModel.HotspotState.Active -> { ActiveHotspotScreen(state = currentState) } @@ -152,6 +160,28 @@ fun HotspotScreen( } } +@Composable +private fun ExistingGroupConfirmation( + onConfirm: () -> Unit, + onCancel: () -> Unit +) { + AlertDialog( + onDismissRequest = onCancel, + title = { Text(stringResource(R.string.hotspot_disconnect_title)) }, + text = { Text(stringResource(R.string.hotspot_disconnect_message)) }, + confirmButton = { + TextButton(onClick = onConfirm) { + Text(stringResource(R.string.hotspot_disconnect_confirm)) + } + }, + dismissButton = { + TextButton(onClick = onCancel) { + Text(stringResource(R.string.cancel)) + } + } + ) +} + @OptIn(ExperimentalPermissionsApi::class) @Composable fun IntroScreen(onStartHotspot: () -> Unit) { diff --git a/app/src/main/java/com/bitchat/android/hotspot/HotspotManager.kt b/app/src/main/java/com/bitchat/android/hotspot/HotspotManager.kt index 8f503fb5..a56182bc 100644 --- a/app/src/main/java/com/bitchat/android/hotspot/HotspotManager.kt +++ b/app/src/main/java/com/bitchat/android/hotspot/HotspotManager.kt @@ -63,6 +63,27 @@ class HotspotManager(private val context: Context) { private var hasNotifiedStarted = false // Track if we've notified the callback private var isReceiverRegistered = false // Track receiver registration to prevent leaks + // Set once our own createGroup command is accepted, and re-checked against every + // group snapshot afterwards. stopHotspot() only calls removeGroup() when this is + // true: removal is device-scoped, so issuing it when the group on the framework + // is not ours could only tear down another app's session (Cast, Android Auto, + // Quick Share). + private var createdGroup = false + + // Framework-reported name of the group this session hosts, once known. Null while + // the group is still forming, when a null snapshot carries no information. + private var hostedGroupName: String? = null + + // Name of the foreign group the user explicitly agreed to disconnect, or null. + // Consent is per-group: a group with a different name asks again. + private var confirmedReplacementName: String? = null + + // stopHotspot() can be reached again while its group query/removal is still in + // flight. Later callers wait for that same teardown instead of releasing the + // Wi-Fi Aware lease early. + private var teardownInProgress = false + private val teardownCallbacks = mutableListOf<() -> Unit>() + // Saved credentials for reconnection private var savedSsid: String? = null private var savedPassword: String? = null @@ -103,8 +124,13 @@ class HotspotManager(private val context: Context) { /** * Start the Wi-Fi P2P hotspot. + * + * @param confirmedReplacementName name of the foreign Wi-Fi Direct group the + * user has confirmed may be disconnected, as previously reported through + * [HotspotCallback.onExistingGroupConflict]. When null (or when the group + * present no longer matches), a foreign group is reported instead of touched. */ - fun startHotspot(callback: HotspotCallback) { + fun startHotspot(callback: HotspotCallback, confirmedReplacementName: String? = null) { if (isStarting) { Log.w(TAG, "Hotspot already starting") return @@ -129,6 +155,7 @@ class HotspotManager(private val context: Context) { } this.callback = callback + this.confirmedReplacementName = confirmedReplacementName isStarting = true Log.d(TAG, "Starting Wi-Fi P2P hotspot") @@ -162,10 +189,20 @@ class HotspotManager(private val context: Context) { /** * Stop the hotspot. + * + * @param onTeardownComplete invoked once the framework has acknowledged the + * removal of our group (or immediately when this session created none). Lets + * the caller hold the Wi-Fi Aware radio back until the P2P group is gone. */ - fun stopHotspot() { + fun stopHotspot(onTeardownComplete: (() -> Unit)? = null) { Log.d(TAG, "Stopping hotspot") + onTeardownComplete?.let(teardownCallbacks::add) + if (teardownInProgress) { + Log.d(TAG, "Teardown already in progress; chaining completion") + return + } + isStarting = false hasNotifiedStarted = false @@ -177,19 +214,28 @@ class HotspotManager(private val context: Context) { val staleChannel = channel channel = null - if (staleChannel != null) { - wifiP2pManager?.removeGroup(staleChannel, object : ActionListener { - override fun onSuccess() { - Log.d(TAG, "Group removed successfully") - // Nothing of ours is left for a later run to clean up. - ownedGroupName = null - closeChannel(staleChannel) - } - override fun onFailure(reason: Int) { - Log.w(TAG, "Failed to remove group: $reason") - closeChannel(staleChannel) - } - }) + val hadOwnGroup = createdGroup + val expectedGroupName = hostedGroupName ?: if ( + Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q + ) { + savedSsid + } else { + null + } + createdGroup = false + hostedGroupName = null + + var teardownAction: (() -> Unit)? = null + if (staleChannel != null && hadOwnGroup) { + teardownInProgress = true + teardownAction = { + removeOwnGroupIfStillPresent(staleChannel, expectedGroupName) + } + } else if (staleChannel != null) { + // This session created nothing, so there is nothing of ours to remove. + // removeGroup() here is exactly the bug this change fixes: device-scoped + // removal would disconnect whatever group another app has running. + closeChannel(staleChannel) } // Release locks @@ -209,6 +255,84 @@ class HotspotManager(private val context: Context) { currentGroup = null callback = null + + if (teardownAction != null) { + teardownAction.invoke() + } else { + finishTeardown() + } + } + + /** + * Re-check the device-scoped group immediately before removing it. The last poll + * is only a snapshot: our group may have disappeared and another app may have + * claimed Wi-Fi Direct before stop was requested. + */ + @SuppressLint("MissingPermission") + private fun removeOwnGroupIfStillPresent(ch: Channel, expectedGroupName: String?) { + val manager = wifiP2pManager ?: run { + closeChannel(ch) + finishTeardown() + return + } + + try { + manager.requestGroupInfo(ch) { group -> + val stillOurs = HotspotStartupPolicy.isExpectedHostedGroup( + existingGroupName = group?.networkName, + isGroupOwner = group?.isGroupOwner == true, + expectedGroupName = expectedGroupName + ) + if (!stillOurs) { + Log.i( + TAG, + "Current group '${group?.networkName}' is not ours " + + "('$expectedGroupName'); leaving it alone" + ) + closeChannel(ch) + finishTeardown() + return@requestGroupInfo + } + + try { + manager.removeGroup(ch, object : ActionListener { + override fun onSuccess() { + Log.d(TAG, "Group removed successfully") + clearOwnedGroupNameIfMatches(expectedGroupName) + closeChannel(ch) + finishTeardown() + } + + override fun onFailure(reason: Int) { + Log.w(TAG, "Failed to remove group: $reason") + closeChannel(ch) + finishTeardown() + } + }) + } catch (e: SecurityException) { + Log.e(TAG, "Wi-Fi permission was revoked while removing the group", e) + closeChannel(ch) + finishTeardown() + } + } + } catch (e: SecurityException) { + Log.e(TAG, "Wi-Fi permission was revoked while confirming group ownership", e) + closeChannel(ch) + finishTeardown() + } + } + + private fun clearOwnedGroupNameIfMatches(removedGroupName: String?) { + if (HotspotStartupPolicy.shouldClearOwnedGroupName(ownedGroupName, removedGroupName)) { + ownedGroupName = null + } + } + + private fun finishTeardown() { + teardownInProgress = false + val callbacks = teardownCallbacks.toList() + teardownCallbacks.clear() + callbacks.forEach { it.invoke() } } /** @@ -305,7 +429,8 @@ class HotspotManager(private val context: Context) { val action = HotspotStartupPolicy.startAction( p2pState = lastP2pState, existingGroupName = existingGroup?.networkName, - ownedGroupName = ownedGroupName + ownedGroupName = ownedGroupName, + confirmedGroupName = confirmedReplacementName ) when (action) { @@ -313,7 +438,19 @@ class HotspotManager(private val context: Context) { Log.w(TAG, "Not attempting group creation: ${action.message}") failStartup(action.message) } - HotspotStartupPolicy.StartAction.Create -> createGroup(attempt) + HotspotStartupPolicy.StartAction.ConfirmReplaceExisting -> { + val name = existingGroup?.networkName + if (name == null) { + // Unreachable while the policy requires a name, but there + // is nothing safe to bind consent to without one. + failStartup(HotspotStartupPolicy.P2P_BUSY_MESSAGE) + } else { + Log.i(TAG, "Existing group '$name' is not ours; asking the user") + reportExistingGroupConflict(name) + } + } + HotspotStartupPolicy.StartAction.Create -> + createGroup(attempt, oldGroupCleared = true) HotspotStartupPolicy.StartAction.RemoveStaleGroupThenCreate -> { Log.w(TAG, "Removing stale group '${existingGroup?.networkName}' before creating") removeStaleGroup(ch, attempt) @@ -327,33 +464,46 @@ class HotspotManager(private val context: Context) { } private fun removeStaleGroup(ch: Channel, attempt: Int) { - wifiP2pManager?.removeGroup(ch, object : ActionListener { - override fun onSuccess() { - if (channel !== ch) return - Log.d(TAG, "Stale group removed") - createGroup(attempt) - } - override fun onFailure(reason: Int) { - if (channel !== ch) return - // Creation may still succeed, and a BUSY reply here backs off as usual. - Log.w(TAG, "Failed to remove stale group: $reason; attempting creation anyway") - createGroup(attempt) - } - }) + try { + wifiP2pManager?.removeGroup(ch, object : ActionListener { + override fun onSuccess() { + if (channel !== ch) return + Log.d(TAG, "Stale group removed") + createGroup(attempt, oldGroupCleared = true) + } + override fun onFailure(reason: Int) { + if (channel !== ch) return + // Creation may still succeed, and a BUSY reply here backs off as usual. + Log.w(TAG, "Failed to remove stale group: $reason; attempting creation anyway") + createGroup(attempt, oldGroupCleared = false) + } + }) + } catch (e: SecurityException) { + Log.e(TAG, "Wi-Fi permission was revoked while removing the existing group", e) + failStartup("A required Wi-Fi or local network permission was revoked. Grant it and try again.") + } } /** * Create Wi-Fi P2P group. + * + * @param oldGroupCleared false when a previous group may still exist (its removal + * just failed). The ownership marker keeps the OLD group's name in that case: + * overwriting it early would make a BUSY retry classify our own stale group as + * foreign and raise a spurious consent dialog. On success the group-info poll + * records the authoritative name anyway. */ @SuppressLint("MissingPermission") - private fun createGroup(attempt: Int) { + private fun createGroup(attempt: Int, oldGroupCleared: Boolean) { val ch = channel ?: return try { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { // Record before the call: if the process dies between creation and the // first group info, the next run still knows this orphan is ours. - ownedGroupName = savedSsid + if (oldGroupCleared) { + ownedGroupName = savedSsid + } // Android 10+: Custom SSID and password val config = WifiP2pConfig.Builder() @@ -376,11 +526,18 @@ class HotspotManager(private val context: Context) { private fun groupActionListener(attempt: Int, requestChannel: Channel) = object : ActionListener { override fun onSuccess() { if (channel !== requestChannel) { + // Ours by construction: this listener only observes our own createGroup. Log.w(TAG, "Removing group created after hotspot was stopped") - wifiP2pManager?.removeGroup(requestChannel, null) + try { + wifiP2pManager?.removeGroup(requestChannel, null) + } catch (e: SecurityException) { + // The orphan stays; the next start recognises it via ownedGroupName. + Log.e(TAG, "Could not remove the late group; permission was revoked", e) + } return } Log.d(TAG, "P2P group created successfully") + createdGroup = true isStarting = false // Don't call onHotspotStarted() yet - wait for group info startGroupInfoPolling() @@ -434,6 +591,17 @@ class HotspotManager(private val context: Context) { cb?.onError(message) } + /** + * A group belonging to another app is up. Stop cleanly — with [createdGroup] + * false the stop path leaves that group untouched — and let the UI ask whether + * starting the hotspot may disconnect it. + */ + private fun reportExistingGroupConflict(groupName: String) { + val cb = callback + stopHotspot() + cb?.onExistingGroupConflict(groupName) + } + /** * Start polling for group info to track connected clients. */ @@ -472,29 +640,52 @@ class HotspotManager(private val context: Context) { try { wifiP2pManager?.requestGroupInfo(ch) { group -> - if (group != null) { - currentGroup = group + // A reply arriving after the hotspot stopped must not revive any + // state the stop just cleared. + if (channel !== ch) return@requestGroupInfo - // Update saved credentials if using system-generated ones - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q) { - savedSsid = group.networkName - savedPassword = group.passphrase - } + reconcileGroupOwnership(group) - // Authoritative name straight from the framework - group.networkName?.let { ownedGroupName = it } - - // Notify callback on FIRST successful group info retrieval - if (!hasNotifiedStarted) { - hasNotifiedStarted = true - Log.d(TAG, "Group info received, notifying callback") - callback?.onHotspotStarted() - } else { - // Subsequent updates - callback?.onConnectionInfoUpdated(getConnectionInfo()) - } - } else { + if (group == null) { Log.w(TAG, "requestGroupInfo returned null group") + return@requestGroupInfo + } + + if (!isOurHostedGroup(group)) { + // Someone else's group is on the radio. Reading anything from it + // — its name, its credentials, its client count — would report + // another app's session as our hotspot, and recording its name + // would let the next start remove it without asking. + Log.w( + TAG, + "Observed group '${group.networkName}' is not the one we created" + ) + return@requestGroupInfo + } + + currentGroup = group + + // Update saved credentials if using system-generated ones + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q) { + savedSsid = group.networkName + savedPassword = group.passphrase + } + + // Authoritative name straight from the framework, for the group we + // just confirmed is ours. + group.networkName?.let { + hostedGroupName = it + ownedGroupName = it + } + + // Notify callback on FIRST successful group info retrieval + if (!hasNotifiedStarted) { + hasNotifiedStarted = true + Log.d(TAG, "Group info received, notifying callback") + callback?.onHotspotStarted() + } else { + // Subsequent updates + callback?.onConnectionInfoUpdated(getConnectionInfo()) } } } catch (e: SecurityException) { @@ -503,6 +694,63 @@ class HotspotManager(private val context: Context) { } } + /** + * Is this snapshot the group this session created? + * + * `isGroupOwner` cannot answer that on its own: it reports that *this device* + * hosts the group, which is equally true of an autonomous group another app + * created here. Above Q we chose the network name, so it identifies our group + * exactly. Below Q the framework names it, and the first snapshot after our own + * createGroup succeeded is the only evidence available — after that the name is + * fixed, and a group answering to a different one is not ours. + */ + private fun isOurHostedGroup(group: WifiP2pGroup): Boolean { + if (!group.isGroupOwner) return false + val name = group.networkName ?: return false + + hostedGroupName?.let { return name == it } + + return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + name == savedSsid + } else { + true + } + } + + /** + * Keep [createdGroup] honest about what is actually on the framework. + * + * Our group can disappear without us — Wi-Fi toggled, another app issuing its own + * device-scoped removeGroup(), a driver reset — and another app can then create + * one in its place. Believing the group present is still ours would make stop + * remove that replacement, the exact disruption consent exists to prevent. + * + * Reconciled only once the framework has named our group: before that a null + * snapshot means the group is still forming, not that it is gone. Losing the flag + * to a transient null is safe in a way that keeping it is not — the group we + * created is then left behind, and the next start recognises it by name and + * removes it silently. + */ + private fun reconcileGroupOwnership(group: WifiP2pGroup?) { + val expectedName = hostedGroupName ?: if ( + Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q + ) { + savedSsid + } else { + null + } ?: return + + // A null snapshot is normal before the configured group appears. A non-null + // group with a different name is positive evidence that ours was replaced. + if (hostedGroupName == null && group == null) return + + val stillOurs = group != null && isOurHostedGroup(group) + if (createdGroup && !stillOurs) { + Log.w(TAG, "Group '$expectedName' is no longer ours; leaving what is present alone") + } + createdGroup = stillOurs + } + /** * Acquire WakeLock and WifiLock to keep hotspot active. */ @@ -618,6 +866,14 @@ class HotspotManager(private val context: Context) { interface HotspotCallback { fun onHotspotStarted() fun onConnectionInfoUpdated(info: ConnectionInfo?) + + /** + * A Wi-Fi Direct group belonging to another app is active and the caller has + * not confirmed replacing it. Ask the user, then retry with this name as + * `confirmedReplacementName` if they accept. Nothing was disturbed. + */ + fun onExistingGroupConflict(groupName: String) + fun onError(message: String) } } diff --git a/app/src/main/java/com/bitchat/android/hotspot/HotspotStartupPolicy.kt b/app/src/main/java/com/bitchat/android/hotspot/HotspotStartupPolicy.kt index 48284d0a..80d6f4ee 100644 --- a/app/src/main/java/com/bitchat/android/hotspot/HotspotStartupPolicy.kt +++ b/app/src/main/java/com/bitchat/android/hotspot/HotspotStartupPolicy.kt @@ -19,8 +19,6 @@ internal object HotspotStartupPolicy { const val SSID_PREFIX = "DIRECT-BC-" // BC for BitChat const val P2P_UNSUPPORTED_MESSAGE = "Wi-Fi Direct is not supported on this device." - const val FOREIGN_GROUP_MESSAGE = - "Another app is using Wi-Fi Direct. Close it and try again." const val P2P_BUSY_MESSAGE = "Wi-Fi Direct is busy. Please try again in a moment." const val GENERIC_FAILURE_MESSAGE = "Failed to start the hotspot. Please try again." @@ -32,6 +30,10 @@ internal object HotspotStartupPolicy { sealed interface StartAction { data object Create : StartAction data object RemoveStaleGroupThenCreate : StartAction + + /** A group we cannot show is ours is up; ask the user before disturbing it. */ + data object ConfirmReplaceExisting : StartAction + data class Fail(val message: String) : StartAction } @@ -42,29 +44,55 @@ internal object HotspotStartupPolicy { * hosting leaves an orphan behind. The framework rejects createGroup with BUSY * while any group exists, and no amount of retrying clears it. * - * Wi-Fi Direct is shared with Cast, Android Auto and Quick Share, so only groups - * we can show are ours get torn down. + * Wi-Fi Direct is shared with Cast, Android Auto and Quick Share. A group we can + * show is ours is removed silently; anything else needs the user's explicit + * go-ahead before it is touched. Consent is bound to the group it was given + * for: a group with any other name — one that appeared after the dialog, or + * mid-retry — asks again instead of riding on stale approval. * * @param existingGroupName network name of the group already present, or null * @param ownedGroupName last group name this app recorded creating, or null + * @param confirmedGroupName group the user agreed to disconnect, or null */ fun startAction( p2pState: Int?, existingGroupName: String?, - ownedGroupName: String? + ownedGroupName: String?, + confirmedGroupName: String? ): StartAction = when { p2pState == WifiP2pManager.WIFI_P2P_STATE_DISABLED -> StartAction.Fail(P2P_DISABLED_MESSAGE) existingGroupName == null -> StartAction.Create isOurs(existingGroupName, ownedGroupName) -> StartAction.RemoveStaleGroupThenCreate - else -> StartAction.Fail(FOREIGN_GROUP_MESSAGE) + existingGroupName == confirmedGroupName -> StartAction.RemoveStaleGroupThenCreate + else -> StartAction.ConfirmReplaceExisting } /** - * Primary signal is the name we recorded creating. The SSID prefix is only a - * fallback, covering orphans left by builds that predate that record. + * Only the exact name this device recorded creating counts as ours. A prefix + * match is not ownership: this device can be connected to another phone's + * bitchat group — same prefix, their suffix — and silently removing it would + * disconnect that session. An orphan predating the record simply goes through + * the confirmation dialog once. */ private fun isOurs(existingGroupName: String, ownedGroupName: String?): Boolean = - existingGroupName == ownedGroupName || existingGroupName.startsWith(SSID_PREFIX) + ownedGroupName != null && existingGroupName == ownedGroupName + + /** Only an exact owner-role name match authorizes device-scoped removal. */ + fun isExpectedHostedGroup( + existingGroupName: String?, + isGroupOwner: Boolean, + expectedGroupName: String? + ): Boolean = + isGroupOwner && + expectedGroupName != null && + existingGroupName == expectedGroupName + + /** A stale teardown must not erase the ownership marker of a newer session. */ + fun shouldClearOwnedGroupName( + storedGroupName: String?, + removedGroupName: String? + ): Boolean = + removedGroupName != null && storedGroupName == removedGroupName /** * @param reason a [WifiP2pManager] failure reason from `ActionListener.onFailure` diff --git a/app/src/main/java/com/bitchat/android/hotspot/HotspotViewModel.kt b/app/src/main/java/com/bitchat/android/hotspot/HotspotViewModel.kt index 581b1d99..73f9faa4 100644 --- a/app/src/main/java/com/bitchat/android/hotspot/HotspotViewModel.kt +++ b/app/src/main/java/com/bitchat/android/hotspot/HotspotViewModel.kt @@ -1,6 +1,8 @@ package com.bitchat.android.hotspot import android.app.Application +import android.os.Handler +import android.os.Looper import android.util.Log import androidx.lifecycle.AndroidViewModel import androidx.lifecycle.viewModelScope @@ -18,6 +20,10 @@ class HotspotViewModel(application: Application) : AndroidViewModel(application) companion object { private const val TAG = "HotspotViewModel" + + // Upper bound on waiting for the framework to acknowledge group removal + // before the Wi-Fi Aware lease is released anyway. + private const val TEARDOWN_FALLBACK_MILLIS = 10_000L } private val _state = MutableStateFlow(HotspotState.Intro) @@ -25,12 +31,25 @@ class HotspotViewModel(application: Application) : AndroidViewModel(application) private var hotspotManager: HotspotManager? = null private var webServer: ApkWebServer? = null + + /** APK waiting on the user's answer to the disconnect confirmation. */ + private var pendingApk: File? = null + + /** Group the pending confirmation is about; consent binds to this name only. */ + private var pendingGroupName: String? = null + + /** Once-releasable radio claim owned by the current hotspot session. */ + private var awareLease: WifiAwareController.HotspotLease? = null private val context = application.applicationContext /** * Start the hotspot with the provided APK file. */ fun startHotspot(apkFile: File) { + startHotspot(apkFile, confirmedGroupName = null) + } + + private fun startHotspot(apkFile: File, confirmedGroupName: String?) { if (_state.value is HotspotState.Starting || _state.value is HotspotState.Active) { Log.w(TAG, "Hotspot already starting or active") return @@ -43,7 +62,7 @@ class HotspotViewModel(application: Application) : AndroidViewModel(application) try { // Wi-Fi Aware holds a NAN interface that blocks the P2P one; release it // first or every createGroup comes back BUSY. Restored when we stop. - WifiAwareController.holdForHotspot() + awareLease = WifiAwareController.acquireHotspotLease() // Start hotspot val manager = HotspotManager(context) @@ -94,10 +113,21 @@ class HotspotViewModel(application: Application) : AndroidViewModel(application) } } + override fun onExistingGroupConflict(groupName: String) { + viewModelScope.launch { + // Nothing was disturbed; the manager already stopped + // itself. Release our resources and ask the user. + teardown() + pendingApk = apkFile + pendingGroupName = groupName + _state.value = HotspotState.ConfirmDisconnect + } + } + override fun onError(message: String) { viewModelScope.launch { failWith(message) } } - }) + }, confirmedGroupName) } catch (e: Exception) { Log.e(TAG, "Error starting hotspot", e) @@ -106,11 +136,34 @@ class HotspotViewModel(application: Application) : AndroidViewModel(application) } } + /** The user agreed that starting may disconnect the existing Wi-Fi Direct group. */ + fun confirmDisconnectAndStart() { + if (_state.value !is HotspotState.ConfirmDisconnect) return + val apk = pendingApk ?: return + val groupName = pendingGroupName + pendingApk = null + pendingGroupName = null + startHotspot(apk, confirmedGroupName = groupName) + } + + fun cancelDisconnect() { + // A tap landing late (e.g. through an exit animation) must not tear down + // the session a just-processed confirmation is starting. + if (_state.value !is HotspotState.ConfirmDisconnect) return + pendingApk = null + pendingGroupName = null + // The conflict path already tore down; this only covers a stray state. + teardown() + _state.value = HotspotState.Intro + } + /** * Stop the hotspot and web server. */ fun stopHotspot() { Log.d(TAG, "Stopping hotspot") + pendingApk = null + pendingGroupName = null teardown() _state.value = HotspotState.Intro } @@ -125,6 +178,8 @@ class HotspotViewModel(application: Application) : AndroidViewModel(application) */ private fun failWith(message: String) { Log.e(TAG, "Hotspot failed: $message") + pendingApk = null + pendingGroupName = null teardown() _state.value = HotspotState.Error(message) } @@ -134,10 +189,34 @@ class HotspotViewModel(application: Application) : AndroidViewModel(application) webServer?.stopServer() webServer = null - hotspotManager?.stopHotspot() + val manager = hotspotManager hotspotManager = null - WifiAwareController.releaseHotspotHold() + // Nothing of ours to hand back. An earlier teardown may already have passed + // its once-releasable lease to the manager's completion callback. + val lease = awareLease ?: return + awareLease = null + + if (manager == null) { + lease.close() + return + } + + // Close the lease only when the manager has finished removing our group. + // Restoring Wi-Fi Aware earlier recreates the NAN/P2P radio contention the + // lease exists to prevent. The lease is idempotent, so a duplicate or late + // completion cannot release a newer session's claim. + manager.stopHotspot { lease.close() } + + // A framework that never acknowledges the removal must not pin Wi-Fi Aware + // down for the life of the process. close() is idempotent and this lease + // belongs to this session alone, so whichever path runs second is a no-op. + // A plain handler rather than viewModelScope: onCleared() cancels the scope + // right when this teardown may be running. + Handler(Looper.getMainLooper()).postDelayed( + { lease.close() }, + TEARDOWN_FALLBACK_MILLIS + ) } /** @@ -160,6 +239,10 @@ class HotspotViewModel(application: Application) : AndroidViewModel(application) sealed class HotspotState { object Intro : HotspotState() object Starting : HotspotState() + + /** A foreign Wi-Fi Direct group is up; waiting for the user's go-ahead. */ + object ConfirmDisconnect : HotspotState() + data class Active( val ssid: String, val password: String, 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 ccb8b55f..d5a694f1 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 @@ -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() + } } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 18762913..88c495c1 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -275,6 +275,9 @@ Create Wi-Fi hotspot to share offline Share via Quick Share Use standard Android sharing + Disconnect current Wi-Fi Direct connection? + Starting the hotspot will disconnect the current Wi-Fi Direct connection. This may interrupt Cast, Android Auto, Quick Share, or another nearby connection. + Disconnect and start Install Received APK diff --git a/app/src/test/kotlin/com/bitchat/android/hotspot/HotspotStartupPolicyTest.kt b/app/src/test/kotlin/com/bitchat/android/hotspot/HotspotStartupPolicyTest.kt index bb91b072..4cc68492 100644 --- a/app/src/test/kotlin/com/bitchat/android/hotspot/HotspotStartupPolicyTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/hotspot/HotspotStartupPolicyTest.kt @@ -2,6 +2,7 @@ package com.bitchat.android.hotspot import android.net.wifi.p2p.WifiP2pManager import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test @@ -76,49 +77,83 @@ class HotspotStartupPolicyTest { val action = HotspotStartupPolicy.startAction( p2pState = WifiP2pManager.WIFI_P2P_STATE_ENABLED, existingGroupName = "DIRECT-BC-CUF6EN63", - ownedGroupName = "DIRECT-BC-CUF6EN63" + ownedGroupName = "DIRECT-BC-CUF6EN63", + confirmedGroupName = null ) assertEquals(HotspotStartupPolicy.StartAction.RemoveStaleGroupThenCreate, action) } @Test - fun `another app's group is left alone`() { + fun `another app's group requires the user's confirmation`() { val action = HotspotStartupPolicy.startAction( p2pState = WifiP2pManager.WIFI_P2P_STATE_ENABLED, existingGroupName = "DIRECT-xY-Chromecast", - ownedGroupName = "DIRECT-BC-CUF6EN63" + ownedGroupName = "DIRECT-BC-CUF6EN63", + confirmedGroupName = null ) - assertEquals( - HotspotStartupPolicy.StartAction.Fail(HotspotStartupPolicy.FOREIGN_GROUP_MESSAGE), - action - ) + assertEquals(HotspotStartupPolicy.StartAction.ConfirmReplaceExisting, action) } @Test - fun `an orphan from before we recorded ownership is still recognised by its prefix`() { + fun `a confirmed replacement removes the confirmed foreign group`() { + val action = HotspotStartupPolicy.startAction( + p2pState = WifiP2pManager.WIFI_P2P_STATE_ENABLED, + existingGroupName = "DIRECT-xY-Chromecast", + ownedGroupName = null, + confirmedGroupName = "DIRECT-xY-Chromecast" + ) + + assertEquals(HotspotStartupPolicy.StartAction.RemoveStaleGroupThenCreate, action) + } + + @Test + fun `consent for one group does not authorize removing a different one`() { + val action = HotspotStartupPolicy.startAction( + p2pState = WifiP2pManager.WIFI_P2P_STATE_ENABLED, + existingGroupName = "DIRECT-aB-QuickShare", + ownedGroupName = null, + confirmedGroupName = "DIRECT-xY-Chromecast" + ) + + assertEquals(HotspotStartupPolicy.StartAction.ConfirmReplaceExisting, action) + } + + @Test + fun `an unrecorded orphan needs confirmation even with our prefix`() { val action = HotspotStartupPolicy.startAction( p2pState = WifiP2pManager.WIFI_P2P_STATE_ENABLED, existingGroupName = "DIRECT-BC-OLDGROUP", - ownedGroupName = null + ownedGroupName = null, + confirmedGroupName = null ) - assertEquals(HotspotStartupPolicy.StartAction.RemoveStaleGroupThenCreate, action) + assertEquals(HotspotStartupPolicy.StartAction.ConfirmReplaceExisting, action) } @Test - fun `a foreign group is left alone even when we recorded nothing`() { + fun `another phone's bitchat group is not treated as ours by its prefix`() { + val action = HotspotStartupPolicy.startAction( + p2pState = WifiP2pManager.WIFI_P2P_STATE_ENABLED, + existingGroupName = "DIRECT-BC-THEIRS99", + ownedGroupName = "DIRECT-BC-CUF6EN63", + confirmedGroupName = null + ) + + assertEquals(HotspotStartupPolicy.StartAction.ConfirmReplaceExisting, action) + } + + @Test + fun `a foreign group needs confirmation even when we recorded nothing`() { val action = HotspotStartupPolicy.startAction( p2pState = WifiP2pManager.WIFI_P2P_STATE_ENABLED, existingGroupName = "DIRECT-xY-Chromecast", - ownedGroupName = null + ownedGroupName = null, + confirmedGroupName = null ) - assertEquals( - HotspotStartupPolicy.StartAction.Fail(HotspotStartupPolicy.FOREIGN_GROUP_MESSAGE), - action - ) + assertEquals(HotspotStartupPolicy.StartAction.ConfirmReplaceExisting, action) } @Test @@ -126,18 +161,35 @@ class HotspotStartupPolicyTest { val action = HotspotStartupPolicy.startAction( p2pState = WifiP2pManager.WIFI_P2P_STATE_ENABLED, existingGroupName = null, - ownedGroupName = null + ownedGroupName = null, + confirmedGroupName = null ) assertEquals(HotspotStartupPolicy.StartAction.Create, action) } + @Test + fun `confirmation does not skip the disabled-P2P check`() { + val action = HotspotStartupPolicy.startAction( + p2pState = WifiP2pManager.WIFI_P2P_STATE_DISABLED, + existingGroupName = "DIRECT-xY-Chromecast", + ownedGroupName = null, + confirmedGroupName = "DIRECT-xY-Chromecast" + ) + + assertEquals( + HotspotStartupPolicy.StartAction.Fail(HotspotStartupPolicy.P2P_DISABLED_MESSAGE), + action + ) + } + @Test fun `disabled P2P fails before touching any existing group`() { val action = HotspotStartupPolicy.startAction( p2pState = WifiP2pManager.WIFI_P2P_STATE_DISABLED, existingGroupName = "DIRECT-BC-CUF6EN63", - ownedGroupName = "DIRECT-BC-CUF6EN63" + ownedGroupName = "DIRECT-BC-CUF6EN63", + confirmedGroupName = null ) assertEquals( @@ -167,4 +219,52 @@ class HotspotStartupPolicyTest { assertTrue(decision is HotspotStartupPolicy.Decision.Fail) } -} \ No newline at end of file + + @Test + fun `only the expected hosted group may be removed on stop`() { + assertTrue( + HotspotStartupPolicy.isExpectedHostedGroup( + existingGroupName = "DIRECT-BC-OURS1234", + isGroupOwner = true, + expectedGroupName = "DIRECT-BC-OURS1234" + ) + ) + assertFalse( + HotspotStartupPolicy.isExpectedHostedGroup( + existingGroupName = "DIRECT-xY-Cast", + isGroupOwner = true, + expectedGroupName = "DIRECT-BC-OURS1234" + ) + ) + assertFalse( + HotspotStartupPolicy.isExpectedHostedGroup( + existingGroupName = "DIRECT-BC-OURS1234", + isGroupOwner = false, + expectedGroupName = "DIRECT-BC-OURS1234" + ) + ) + assertFalse( + HotspotStartupPolicy.isExpectedHostedGroup( + existingGroupName = "DIRECT-BC-OURS1234", + isGroupOwner = true, + expectedGroupName = null + ) + ) + } + + @Test + fun `an old teardown cannot clear a newer ownership marker`() { + assertTrue( + HotspotStartupPolicy.shouldClearOwnedGroupName( + storedGroupName = "DIRECT-BC-OLD12345", + removedGroupName = "DIRECT-BC-OLD12345" + ) + ) + assertFalse( + HotspotStartupPolicy.shouldClearOwnedGroupName( + storedGroupName = "DIRECT-BC-NEW67890", + removedGroupName = "DIRECT-BC-OLD12345" + ) + ) + } +} diff --git a/app/src/test/kotlin/com/bitchat/android/wifiaware/WifiAwareHotspotHoldTest.kt b/app/src/test/kotlin/com/bitchat/android/wifiaware/WifiAwareHotspotHoldTest.kt new file mode 100644 index 00000000..7805b9ad --- /dev/null +++ b/app/src/test/kotlin/com/bitchat/android/wifiaware/WifiAwareHotspotHoldTest.kt @@ -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() + ) + } +}