From 9b7f1631f0765fa208ff4f9a5a358661eb816282 Mon Sep 17 00:00:00 2001 From: zarzet Date: Fri, 10 Jul 2026 09:48:52 +0700 Subject: [PATCH] fix(native-worker): keep batches and retries intact across seam races - Verification challenge no longer abandons the batch: batch mates the worker cancel would mark skipped are requeued and fenced from later snapshot applications, and the adoption loop restarts the queue for requeued items when the run ends - Pause vs cancel race: a cancelled result with no pause flag set waits up to 1.5s for the pause intent to land on the main looper before being classified as a permanent skip - Stale Go cancel flag: new ResetDownloadCancel export (wired through Android and iOS bridges) drops a pre-registered cancel with no active download; retryItem/retryAllFailed call it so the first retry of a cancelled-before-start item no longer aborts instantly - Reconcile-loop errors now cancel the native worker and clear the run id before marking items failed, instead of leaving the service downloading a batch the UI already wrote off --- .../com/zarz/spotiflac/DownloadService.kt | 21 +++++++ .../kotlin/com/zarz/spotiflac/MainActivity.kt | 7 +++ go_backend/cancel.go | 16 +++++ go_backend/exports_download.go | 7 +++ .../extension_runtime_supplement_test.go | 29 ++++++++++ ios/Runner/AppDelegate.swift | 8 ++- lib/providers/download_queue_provider.dart | 58 +++++++++++++++++++ lib/services/platform_bridge.dart | 6 ++ 8 files changed, 151 insertions(+), 1 deletion(-) diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/DownloadService.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/DownloadService.kt index cfe1d60e..3e992d90 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/DownloadService.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/DownloadService.kt @@ -755,6 +755,27 @@ class DownloadService : Service() { writeNativeAlbumReplayGainIfComplete() } else { val errorType = result.optString("error_type") + if (errorType == "cancelled" && + !nativeWorkerPaused && + !nativeWorkerCancelRequested && + generation == nativeWorkerGeneration + ) { + // A pause from Dart cancels the in-flight Go + // download directly but delivers the pause flag + // via a startService intent through the main + // looper; the download can unwind first. Give the + // flag a moment to settle before classifying this + // cancellation as a permanent skip. + var waitedMs = 0L + while (waitedMs < 1500 && + !nativeWorkerPaused && + !nativeWorkerCancelRequested && + generation == nativeWorkerGeneration + ) { + delay(100) + waitedMs += 100 + } + } if (errorType == "cancelled" && nativeWorkerPaused && !nativeWorkerCancelRequested) { updateNativeWorkerItem(request.itemId) { it.status = "queued" diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt index f6453496..53d71a58 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt @@ -2308,6 +2308,13 @@ class MainActivity: FlutterFragmentActivity() { } result.success(null) } + "resetDownloadCancel" -> { + val itemId = call.argument("item_id") ?: "" + withContext(Dispatchers.IO) { + Gobackend.resetDownloadCancel(itemId) + } + result.success(null) + } "setDownloadDirectory" -> { val path = call.argument("path") ?: "" withContext(Dispatchers.IO) { diff --git a/go_backend/cancel.go b/go_backend/cancel.go index 10b7e2e0..e9a3a247 100644 --- a/go_backend/cancel.go +++ b/go_backend/cancel.go @@ -91,6 +91,22 @@ func isDownloadCancelled(itemID string) bool { return canceled } +// resetDownloadCancel removes a cancellation entry that has no active +// download attached (refs <= 0). Such entries exist to catch an item that is +// just about to start, but if the item never starts the flag lingers and the +// next explicit retry would consume it and abort immediately. +func resetDownloadCancel(itemID string) { + if itemID == "" { + return + } + + cancelMu.Lock() + if entry, ok := cancelMap[itemID]; ok && entry.refs <= 0 { + delete(cancelMap, itemID) + } + cancelMu.Unlock() +} + func clearDownloadCancel(itemID string) { if itemID == "" { return diff --git a/go_backend/exports_download.go b/go_backend/exports_download.go index 906b02a1..c9c29176 100644 --- a/go_backend/exports_download.go +++ b/go_backend/exports_download.go @@ -419,6 +419,13 @@ func CancelDownload(itemID string) { cancelDownload(itemID) } +// ResetDownloadCancel drops a pre-registered cancellation flag for an item +// with no active download, so a user-initiated retry does not consume a stale +// cancel and abort instantly. Entries with live references are left alone. +func ResetDownloadCancel(itemID string) { + resetDownloadCancel(itemID) +} + func CleanupConnections() { CloseIdleConnections() } diff --git a/go_backend/extension_runtime_supplement_test.go b/go_backend/extension_runtime_supplement_test.go index b3ebf5a6..41994b91 100644 --- a/go_backend/extension_runtime_supplement_test.go +++ b/go_backend/extension_runtime_supplement_test.go @@ -267,6 +267,35 @@ func TestFileDownloadFailureLeavesNoFinalFile(t *testing.T) { } } +func TestResetDownloadCancelDropsStaleFlag(t *testing.T) { + const itemID = "reset-cancel-item" + + // A cancel issued while nothing is downloading pre-registers a flag that + // the next attempt would consume and abort; reset must drop it. + cancelDownload(itemID) + if !isDownloadCancelled(itemID) { + t.Fatal("expected pre-registered cancel flag") + } + resetDownloadCancel(itemID) + if isDownloadCancelled(itemID) { + t.Fatal("expected stale cancel flag to be dropped") + } + + // A cancel attached to an active download must survive reset. + ctx := initDownloadCancel(itemID) + cancelDownload(itemID) + resetDownloadCancel(itemID) + if !isDownloadCancelled(itemID) { + t.Fatal("expected active cancel to survive reset") + } + select { + case <-ctx.Done(): + default: + t.Fatal("expected cancelled context") + } + clearDownloadCancel(itemID) +} + func TestParseExtensionTrackValueExplicit(t *testing.T) { vm := goja.New() diff --git a/ios/Runner/AppDelegate.swift b/ios/Runner/AppDelegate.swift index e14b4dc4..10e0c79d 100644 --- a/ios/Runner/AppDelegate.swift +++ b/ios/Runner/AppDelegate.swift @@ -431,7 +431,13 @@ import Gobackend let itemId = args["item_id"] as! String GobackendCancelDownload(itemId) return nil - + + case "resetDownloadCancel": + let args = call.arguments as! [String: Any] + let itemId = args["item_id"] as! String + GobackendResetDownloadCancel(itemId) + return nil + case "setDownloadDirectory": let args = call.arguments as! [String: Any] let path = args["path"] as! String diff --git a/lib/providers/download_queue_provider.dart b/lib/providers/download_queue_provider.dart index a5d94ef7..832c25fc 100644 --- a/lib/providers/download_queue_provider.dart +++ b/lib/providers/download_queue_provider.dart @@ -2566,6 +2566,14 @@ class DownloadQueueNotifier extends Notifier { } _log.i('Retrying item: ${item.track.name} (id: $id)'); + // A cancel issued while the item never started leaves a pre-registered + // flag in the Go backend that the next attempt would consume and abort + // instantly; the user asked for a retry, so drop it first. + unawaited( + PlatformBridge.resetDownloadCancel(id).catchError((Object e) { + _log.w('Failed to reset cancel flag for $id: $e'); + }), + ); _locallyCancelledItemIds.remove(id); _verificationRetriedItemIds.removeWhere( (retryKey) => retryKey == id || retryKey.startsWith('$id::'), @@ -2619,6 +2627,13 @@ class DownloadQueueNotifier extends Notifier { } _log.i('Retrying ${failedIds.length} failed download(s)'); + for (final id in failedIds) { + unawaited( + PlatformBridge.resetDownloadCancel(id).catchError((Object e) { + _log.w('Failed to reset cancel flag for $id: $e'); + }), + ); + } _locallyCancelledItemIds.removeAll(failedIds); _pausePendingItemIds.removeAll(failedIds); @@ -4055,6 +4070,14 @@ class DownloadQueueNotifier extends Notifier { ); if (snapshot['is_running'] != true) { await _clearNativeWorkerRunId(runId); + // Items may have been requeued during reconciliation (e.g. batch + // mates of a verification challenge); hand them to the queue. + if (!state.isPaused && + state.items.any( + (item) => item.status == DownloadStatus.queued, + )) { + Future.microtask(() => _processQueue()); + } break; } await Future.delayed(const Duration(seconds: 1)); @@ -4174,6 +4197,16 @@ class DownloadQueueNotifier extends Notifier { return false; } _log.e('Android native worker failed: $e', e, stack); + // The native worker keeps downloading on its own; without cancelling + // it here the batch we just marked failed would continue in the + // background, its completions never reconciled, and the Dart queue + // could even restart alongside it. + try { + await PlatformBridge.cancelNativeDownloadWorker(); + } catch (cancelError) { + _log.w('Failed to cancel native worker after error: $cancelError'); + } + await _clearNativeWorkerRunId(runId); for (final item in queuedItems) { final current = _findItemById(item.id); if (current == null || @@ -4543,11 +4576,36 @@ class DownloadQueueNotifier extends Notifier { _log.i( 'Android native worker requires verification for ${current.track.name}; switching back to interactive queue', ); + // Cancelling the native worker marks every remaining batch item + // "skipped" on the native side. Those items did nothing wrong — + // remember them now and requeue them below so the batch resumes + // after the challenge instead of being abandoned. + final pendingBatchIds = []; + for (final pendingId in contexts.keys) { + if (pendingId == itemId || reconciledIds.contains(pendingId)) { + continue; + } + final pending = _findItemById(pendingId); + if (pending == null) continue; + if (pending.status == DownloadStatus.queued || + pending.status == DownloadStatus.downloading || + pending.status == DownloadStatus.finalizing) { + pendingBatchIds.add(pendingId); + } + } try { await PlatformBridge.cancelNativeDownloadWorker(); } catch (e) { _log.w('Failed to cancel native worker before verification: $e'); } + for (final pendingId in pendingBatchIds) { + reconciledIds.add(pendingId); + updateItemStatus( + pendingId, + DownloadStatus.queued, + progress: 0.0, + ); + } await _handleVerificationRequiredDownload( current, errorMsg, diff --git a/lib/services/platform_bridge.dart b/lib/services/platform_bridge.dart index 9aff0482..53a367df 100644 --- a/lib/services/platform_bridge.dart +++ b/lib/services/platform_bridge.dart @@ -541,6 +541,12 @@ class PlatformBridge { await _channel.invokeMethod('cancelDownload', {'item_id': itemId}); } + /// Drops a stale pre-registered cancel flag for an item with no active + /// download, so a user-initiated retry does not abort instantly. + static Future resetDownloadCancel(String itemId) async { + await _channel.invokeMethod('resetDownloadCancel', {'item_id': itemId}); + } + static Future setDownloadDirectory(String path) async { await _channel.invokeMethod('setDownloadDirectory', {'path': path}); }