From 066fafbb81d645515c8100d37db1b5149b5d0e44 Mon Sep 17 00:00:00 2001 From: QIU Date: Fri, 14 Aug 2026 13:07:53 +0000 Subject: [PATCH] fix(data): use Mutex to queue calculatePasses, not drop calls The previous guard (if (_isCalculating.value) return) silently dropped concurrent calls. Every call carries filter settings the user just applied, so a dropped one left the list showing results for the previous filter: User clicks 'Apply' with elevation>=5 -> UI updates to show elevation>=5 -> calculatePasses(elevation>=5) called -> but if _isCalculating=true, return immediately -> list still shows elevation>=30 results The guard window is wide: delay(1000) + real calculation time (hundreds of ms to seconds), exactly when the progress indicator spins and users naturally interact again. Mutex serializes calls instead: the second one queues and eventually runs with its own parameters. This also fixes the original concurrency issue (duplicate parallel calculations) and adds finally {} so a thrown exception cannot leave isCalculating stuck at true (frozen progress indicator). Reverts the regression introduced in the previous attempt to add concurrency protection. --- .../core/data/repository/SatelliteRepo.kt | 88 +++++++++++-------- 1 file changed, 51 insertions(+), 37 deletions(-) diff --git a/core/data/src/main/java/com/rtbishop/look4sat/core/data/repository/SatelliteRepo.kt b/core/data/src/main/java/com/rtbishop/look4sat/core/data/repository/SatelliteRepo.kt index c1a69693..31aec40e 100644 --- a/core/data/src/main/java/com/rtbishop/look4sat/core/data/repository/SatelliteRepo.kt +++ b/core/data/src/main/java/com/rtbishop/look4sat/core/data/repository/SatelliteRepo.kt @@ -35,6 +35,8 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.update +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import java.util.TimeZone @@ -50,6 +52,10 @@ class SatelliteRepo( private val _isCalculating = MutableStateFlow(false) override val isCalculating: StateFlow = _isCalculating + // Serializes pass calculation. Callers queue instead of being dropped: a + // dropped call would silently discard the filter the user just applied. + private val calculationMutex = Mutex() + private val _satellites = MutableStateFlow>(emptyList()) override val satellites: StateFlow> = _satellites @@ -117,47 +123,55 @@ class SatelliteRepo( invertAosTimeWindow: Boolean, modes: List ) { - // Reject concurrent calls to prevent duplicate calculations - if (_isCalculating.value) return - _isCalculating.value = true - // Normalize to the start of the current minute so that coarse 60-second stepping - // in getLeoPass always begins from the same phase, producing stable AOS/LOS times - val normalizedTime = time / 60_000L * 60_000L - val currentSatellites = _satellites.value - withContext(dispatcher) { - val idsWithModes = localStorage.getIdsWithModes(modes) - val stationPos = settingsRepo.stationPosition.value - val filteredSatellites = if (idsWithModes.isEmpty()) { - currentSatellites - } else { - currentSatellites.filter { it.data.catnum in idsWithModes } - } - // Compute passes for each satellite in parallel - val passLists = coroutineScope { - filteredSatellites.map { satellite -> - async { satellite.getPasses(stationPos, normalizedTime, hoursAhead) } - }.awaitAll() - } - // Flatten and filter in a single pass - val timeFuture = normalizedTime + (hoursAhead * 60L * 60L * 1000L) - val newPasses = ArrayList() - for (list in passLists) { - for (pass in list) { - if ( - pass.losTime > time - && pass.aosTime < timeFuture - && pass.maxElevation > minElevation - && (pass.isDeepSpace || isAosInRange(pass.aosTime, aosStartMinute, aosEndMinute, invertAosTimeWindow)) - ) { - newPasses.add(pass) + // Queue behind any in-flight calculation rather than dropping this call: + // every invocation carries filter settings the user just chose, so a + // dropped one leaves the list showing results for the previous filter. + calculationMutex.withLock { + _isCalculating.value = true + try { + // Normalize to the start of the current minute so that coarse 60-second stepping + // in getLeoPass always begins from the same phase, producing stable AOS/LOS times + val normalizedTime = time / 60_000L * 60_000L + val currentSatellites = _satellites.value + withContext(dispatcher) { + val idsWithModes = localStorage.getIdsWithModes(modes) + val stationPos = settingsRepo.stationPosition.value + val filteredSatellites = if (idsWithModes.isEmpty()) { + currentSatellites + } else { + currentSatellites.filter { it.data.catnum in idsWithModes } } + // Compute passes for each satellite in parallel + val passLists = coroutineScope { + filteredSatellites.map { satellite -> + async { satellite.getPasses(stationPos, normalizedTime, hoursAhead) } + }.awaitAll() + } + // Flatten and filter in a single pass + val timeFuture = normalizedTime + (hoursAhead * 60L * 60L * 1000L) + val newPasses = ArrayList() + for (list in passLists) { + for (pass in list) { + if ( + pass.losTime > time + && pass.aosTime < timeFuture + && pass.maxElevation > minElevation + && (pass.isDeepSpace || isAosInRange(pass.aosTime, aosStartMinute, aosEndMinute, invertAosTimeWindow)) + ) { + newPasses.add(pass) + } + } + } + newPasses.sortBy { it.aosTime } + delay(1000) // Simulate loading time for better UX + _passes.update { newPasses } } + } finally { + // finally: a thrown/cancelled calculation must not leave the + // progress indicator spinning forever. + _isCalculating.value = false } - newPasses.sortBy { it.aosTime } - delay(1000) // Simulate loading time for better UX - _passes.update { newPasses } } - _isCalculating.value = false } private fun isAosInRange(