From 05896f354dd2dd132e4eef81d92224e386245ba0 Mon Sep 17 00:00:00 2001 From: Moe Hamade <69801237+moehamade@users.noreply.github.com> Date: Wed, 29 Jul 2026 13:09:04 +0300 Subject: [PATCH] fix: route every hotspot failure through one teardown path Addresses three issues from Codex review on #808. The Wi-Fi Aware hold could be defeated by a race. startIfPossible() does a long stretch of async work between checking the hold and assigning the service, so holdForHotspot() landing in that window left an in-flight start free to resurrect NAN behind the hotspot's back, putting every P2P attempt back on BUSY. The hold is now rechecked before committing the service, and the freshly started service is torn down if the hotspot claimed the radio meanwhile. Startup error paths bypassed cleanup. A web-server failure, a null connection info, or a throw from the outer block stopped the manager but left the Aware hold set, blocking all mesh starts until the user happened to retry or close the screen. Worse, an error after the server had started left it serving the APK on port 9999 -- including after the device reconnected to an ordinary Wi-Fi network -- because only stopHotspot() cleared it. Both follow from the same gap: cleanup lived at the call sites rather than in one place. All failures now go through failWith(), which shares teardown() with stopHotspot() and releases the server, the manager and the hold together. Co-Authored-By: Claude Opus 5 (1M context) --- .../android/hotspot/HotspotViewModel.kt | 38 +++++++++++-------- .../android/wifi-aware/WifiAwareController.kt | 16 +++++++- 2 files changed, 38 insertions(+), 16 deletions(-) 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 47ad1c5e..581b1d99 100644 --- a/app/src/main/java/com/bitchat/android/hotspot/HotspotViewModel.kt +++ b/app/src/main/java/com/bitchat/android/hotspot/HotspotViewModel.kt @@ -57,8 +57,7 @@ class HotspotViewModel(application: Application) : AndroidViewModel(application) // Get connection info val info = manager.getConnectionInfo() if (info == null) { - manager.stopHotspot() - _state.value = HotspotState.Error("Failed to get hotspot connection info") + failWith("Failed to get hotspot connection info") return@launch } @@ -80,8 +79,7 @@ class HotspotViewModel(application: Application) : AndroidViewModel(application) ) } catch (e: Exception) { Log.e(TAG, "Failed to start web server", e) - manager.stopHotspot() - _state.value = HotspotState.Error("Failed to start web server: ${e.message}") + failWith("Failed to start web server: ${e.message}") } } } @@ -97,20 +95,13 @@ class HotspotViewModel(application: Application) : AndroidViewModel(application) } override fun onError(message: String) { - viewModelScope.launch { - Log.e(TAG, "Hotspot error: $message") - // The manager has already torn itself down; give the mesh - // its radio back rather than holding it for a dead hotspot. - WifiAwareController.releaseHotspotHold() - _state.value = HotspotState.Error(message) - } + viewModelScope.launch { failWith(message) } } }) } catch (e: Exception) { Log.e(TAG, "Error starting hotspot", e) - hotspotManager?.stopHotspot() - _state.value = HotspotState.Error(e.message ?: "Unknown error") + failWith(e.message ?: "Unknown error") } } } @@ -120,7 +111,26 @@ class HotspotViewModel(application: Application) : AndroidViewModel(application) */ fun stopHotspot() { Log.d(TAG, "Stopping hotspot") + teardown() + _state.value = HotspotState.Intro + } + /** + * Every failure after the hotspot has been requested must land here. + * + * Skipping any part of this leaves something running that shouldn't be: the web + * server keeps serving the APK on whatever network the device joins next, and the + * Wi-Fi Aware hold blocks the mesh until the user happens to retry or close the + * screen. + */ + private fun failWith(message: String) { + Log.e(TAG, "Hotspot failed: $message") + teardown() + _state.value = HotspotState.Error(message) + } + + /** Releases every resource startHotspot may have acquired. Safe to call twice. */ + private fun teardown() { webServer?.stopServer() webServer = null @@ -128,8 +138,6 @@ class HotspotViewModel(application: Application) : AndroidViewModel(application) hotspotManager = null WifiAwareController.releaseHotspotHold() - - _state.value = HotspotState.Intro } /** 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 13cbd8fe..f64e1527 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 @@ -179,7 +179,7 @@ object WifiAwareController { return } } - if (!_enabled.value) { + if (!_enabled.value || hotspotHold.get()) { synchronized(lifecycleLock) { starting = false } return } @@ -189,6 +189,20 @@ object WifiAwareController { WifiAwareMeshService(ctx) } startedService.startServices() + + // The hotspot can claim the radio at any point during the work above. + // Committing the service now would resurrect NAN behind its back and + // leave every P2P attempt answering BUSY, so drop what we just started. + if (hotspotHold.get()) { + Log.i(TAG, "Hotspot claimed the radio while starting; abandoning Wi-Fi Aware start") + try { startedService.stopServices() } catch (_: Exception) { } + synchronized(lifecycleLock) { + if (service === startedService) service = null + _running.value = false + } + return + } + if (startedService.isRunning()) { synchronized(lifecycleLock) { service = startedService