From c05486b49caba7c642a6b5f894176dd76d147ef5 Mon Sep 17 00:00:00 2001 From: Broque Thomas Date: Mon, 26 Jan 2026 16:19:51 -0800 Subject: [PATCH] Fix race condition preventing failed tracks from being added to wishlist Root cause: When album downloads completed, the frontend immediately closed the modal and called /api/playlists/cleanup_batch, which deleted the batch from memory. The wishlist processing thread (submitted to executor) would then try to access the batch and fail silently because it was already deleted. This explains why: - Wishlist auto-processing worked (different timing/3-second delay) - Manual "Add to Wishlist" button worked (different code path, before downloads) - Album modal failed tracks didn't get added (race condition) The fix prevents batch cleanup until wishlist processing completes: Backend (web_server.py): 1. Mark wishlist_processing_started=True when submitting processing job 2. Mark wishlist_processing_complete=True when processing finishes 3. Block cleanup endpoint if processing in progress (return 202) Frontend (script.js): 4. Handle 202 response and retry cleanup after 2-second delay This eliminates the race condition while maintaining async processing and ensuring all failed/cancelled tracks are properly added to the wishlist. Co-Authored-By: Claude Sonnet 4.5 --- web_server.py | 26 ++++++++++++++++++++++---- webui/static/script.js | 23 +++++++++++++++++++++-- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/web_server.py b/web_server.py index 0729de97..f4369dfd 100644 --- a/web_server.py +++ b/web_server.py @@ -11447,8 +11447,9 @@ def _process_failed_tracks_to_wishlist_exact(batch_id): with tasks_lock: if batch_id in download_batches: download_batches[batch_id]['wishlist_summary'] = completion_summary + download_batches[batch_id]['wishlist_processing_complete'] = True # Phase already set to 'complete' in _on_download_completed - + print(f"✅ [Wishlist Processing] Completed wishlist processing for batch {batch_id}") return completion_summary @@ -11469,6 +11470,7 @@ def _process_failed_tracks_to_wishlist_exact(batch_id): 'total_failed': 0, 'error_message': str(e) } + download_batches[batch_id]['wishlist_processing_complete'] = True except Exception as lock_error: print(f"❌ [Wishlist Processing] Failed to update batch after error: {lock_error}") @@ -11717,7 +11719,10 @@ def _on_download_completed(batch_id, task_id, success=True): print(f"🎉 [Batch Manager] Batch {batch_id} complete - stopping monitor") download_monitor.stop_monitoring(batch_id) - + + # Mark that wishlist processing is starting (prevents premature cleanup) + batch['wishlist_processing_started'] = True + # Process wishlist outside of the lock to prevent threading issues if is_auto_batch: # For auto-initiated batches, handle completion and schedule next cycle @@ -13824,9 +13829,22 @@ def cleanup_batch(): with tasks_lock: # Check if the batch exists before trying to delete if batch_id in download_batches: + batch = download_batches[batch_id] + + # CRITICAL: Don't allow cleanup if wishlist processing is in progress + # This prevents a race condition where cleanup deletes the batch before + # the wishlist processing thread can access it + if batch.get('wishlist_processing_started') and not batch.get('wishlist_processing_complete'): + print(f"⏳ [Cleanup] Batch {batch_id} cleanup deferred - wishlist processing in progress") + return jsonify({ + "success": False, + "error": "Batch cleanup deferred - wishlist processing in progress", + "deferred": True + }), 202 # 202 = Accepted but not yet processed + # Get the list of task IDs before deleting the batch - task_ids_to_remove = download_batches[batch_id].get('queue', []) - + task_ids_to_remove = batch.get('queue', []) + # Delete the batch record del download_batches[batch_id] diff --git a/webui/static/script.js b/webui/static/script.js index 1b0fdaf2..1cb6434b 100644 --- a/webui/static/script.js +++ b/webui/static/script.js @@ -5068,12 +5068,31 @@ async function cleanupDownloadProcess(playlistId) { if (process.batchId) { try { console.log(`🚀 Sending cleanup request to server for batch: ${process.batchId}`); - await fetch('/api/playlists/cleanup_batch', { + const response = await fetch('/api/playlists/cleanup_batch', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ batch_id: process.batchId }) }); - console.log(`✅ Server cleanup completed for batch: ${process.batchId}`); + + // Handle deferred cleanup (202 = wishlist processing in progress) + if (response.status === 202) { + console.log(`⏳ Wishlist processing in progress for batch ${process.batchId}, will retry cleanup in 2s...`); + // Retry cleanup after delay to allow wishlist processing to complete + setTimeout(async () => { + try { + await fetch('/api/playlists/cleanup_batch', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ batch_id: process.batchId }) + }); + console.log(`✅ Delayed cleanup completed for batch: ${process.batchId}`); + } catch (error) { + console.warn(`⚠️ Delayed cleanup failed:`, error); + } + }, 2000); // 2 second delay + } else { + console.log(`✅ Server cleanup completed for batch: ${process.batchId}`); + } } catch (error) { console.warn(`⚠️ Failed to send cleanup request to server:`, error); // Don't show toast for cleanup failures - they're not user-facing