fix: serialize Spotify captures to prevent cross-download audio mixup (v0.29.0) - #48
Merged
Merged
Conversation
… (v0.29.0) A Spotify account has a single active playback stream, but Spotify captures ran in a thread pool with no serialization. Two overlapping /download-by-id requests each started a librespot device (all named the constant flacfetch-capture) and drove playback via the Web API on the same account. The second start_playback() hijacked the account's one playback stream, so the first download's capture pipe recorded the other track's audio while the second starved and timed out -- silently producing input FLACs with the wrong song (job 842948f4: source Maduk - One Last Picture, received Keeno - Shelter from the Storm audio; both Spotify downloads started within 2 seconds). - Serialize Spotify captures process-wide (_SPOTIFY_CAPTURE_LOCK); other sources (YouTube, torrents) still run concurrently. - Unique librespot device name per capture (flacfetch-capture-<uuid>); resolve by that exact name so a stale/overlapping device can't be mistaken for ours. - Fail loudly if the requested track never loads (the track-load result was previously discarded, so capture proceeded regardless). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Use full uuid for the librespot device name (was 8 hex chars / 32 bits; collision could match a stale device the change intends to avoid). - Make intermediate capture files (PCM, librespot log) unique per download via capture_id, and publish the final FLAC via an atomic os.replace() from a per-download temp file. This closes a race where releasing the capture lock before conversion let a same-named concurrent download truncate the PCM mid-conversion or produce a half-written FLAC. - Install shared test patches once in the main thread (ExitStack) instead of inside each worker thread, so patch setup/teardown can't interleave. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address second CodeRabbit pass: - Always delete the per-capture PCM, librespot log, and temp FLAC via a single outer finally, so a failed capture (device not found, track not loaded, timeout) no longer orphans a tens-of-MB PCM on disk (capture files now carry a unique capture_id, so failures previously never got cleaned up). - Pass the per-download device_name into _device_not_found_message so the diagnostic names the device we actually registered (flacfetch-capture-<uuid>) instead of the bare constant. - Add regression test asserting a failed capture leaves no orphan temp files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
beveradb
deleted the
feat/sess-20260901-1844-spotify-concurrency-mixup
branch
September 2, 2026 16:06
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Job
842948f4(source: Maduk - One Last Picture, Spotify) received the wrong input audio — itsinput/Unknown - 4LXnEERKcz4aRC4NCMQJ0x.flacactually contained Keeno - Shelter from the Storm (job12accca1, also a Spotify source made moments earlier).Root cause — confirmed from production logs
Spotify captures ran in a thread pool with no serialization. A Spotify account has a single active playback stream, and every capture registered a librespot Spotify Connect device under the same constant name
flacfetch-captureand drove playback via the Web API on the same account.Production timeline (flacup journal, 2026-08-31):
The second
start_playback()hijacked the account's one playback stream, so job A's capture pipe recorded job B's track, and job B captured nothing and timed out. The wrong audio is undetectable downstream (valid FLAC, right filename).Fix
_SPOTIFY_CAPTURE_LOCK): only one librespot device + playback + capture runs at a time. YouTube and torrent downloads are process/file isolated and remain concurrent. The lock is always released (capture is timeout-bounded with afinallythat stops librespot), so it can't deadlock; queued downloads log how long they waited.flacfetch-capture-<uuid>), resolved by that exact name — a stale/overlapping device can never be mistaken for this download's.os.replace(); a single outerfinallyalways cleans them up (no orphan PCMs on disk).Testing
TestSpotifyConcurrencySerialization: proves two concurrentdownload()calls never overlap their capture, each uses a unique device name, a non-loading track fails, and a failed capture leaves no orphan temp files.Reviewed locally with CodeRabbit (3 passes, all findings addressed).
@coderabbitai ignore