# Fix Duplicate Full-Track Download Race in Client Cache

Priority: **Medium** · Effort: **Low** · Risk: **Low**

## Problem

`M.bulkDownloadStarted` is a per-track flag meant to ensure `downloadAndCacheTrack` runs at most once per track. But the flag is **set inside `downloadAndCacheTrack` after an `await`**, while multiple callers check it **before** that await resolves. When the last two segments complete near-simultaneously — one from the browser's buffered-range scan in `ui.js`, one from the explicit `fetchSegment` in `audioCache.js` — both callers observe `trackCache.size >= SEGMENTS` and both pass the `bulkDownloadStarted` guard before the first caller sets the flag. Result: the same track is fetched in full **twice**, wasting bandwidth and racing on the IndexedDB write.

## Affected Locations

The check+call pattern appears in three places, all racy:

- `public/audioCache.js:99-107` — `checkAndCacheComplete`
- `public/ui.js:148-150` — buffer-segment scan
- `public/audioCache.js:183-187` — inside `fetchSegment`

And the flag is set here:

- `public/audioCache.js:110-112` — inside `downloadAndCacheTrack`, **after** `await M.loadTrackBlob(trackId)`.

## Root Cause

Guard-then-do where the guard write is not atomic with respect to the awaits surrounding it. JavaScript's single-threaded execution means a synchronous "check-and-set" is race-free, but here the set happens after the first `await` inside the function being guarded, so a second caller can enter the function and pass the check before the first caller reaches the set.

## Implementation Plan

### Step 1 — Move the flag set to be synchronous with the check

Refactor so the check and the set happen in the same synchronous tick, before any `await`. Create a single entry point:

```js
// public/audioCache.js
M.maybeStartBulkDownload = function (trackId) {
  if (M.bulkDownloadStarted.get(trackId)) return false;   // already started
  if (M.cachedTracks.has(trackId)) return false;          // already cached
  const trackCache = M.trackCaches.get(trackId);
  if (!trackCache || trackCache.size < SEGMENTS) return false;  // not fully buffered
  // All conditions met — claim the slot synchronously:
  M.bulkDownloadStarted.set(trackId, true);
  // Fire the async work without awaiting here:
  M.downloadAndCacheTrack(trackId).catch((err) => {
    console.warn("[audioCache] bulk download failed for", trackId, err);
    M.bulkDownloadStarted.delete(trackId);   // allow a later retry
  });
  return true;
};
```

Then make `downloadAndCacheTrack` **assume the flag is already set** (remove the guard inside it):

```js
M.downloadAndCacheTrack = async function (trackId) {
  // Precondition: M.bulkDownloadStarted.get(trackId) === true (set by maybeStartBulkDownload)
  const cachedUrl = await M.loadTrackBlob(trackId);
  if (cachedUrl) {                 // already cached under us
    M.cachedTracks.add(trackId);
    M.bulkDownloadStarted.delete(trackId);
    return cachedUrl;
  }
  // ... existing download + TrackStorage.set + URL.createObjectURL logic ...
  M.cachedTracks.add(trackId);
  M.bulkDownloadStarted.delete(trackId);  // clear after success
  M.renderQueue && M.renderQueue();
  M.renderLibrary && M.renderLibrary();
  return blobUrl;
};
```

### Step 2 — Replace all three call sites

Each caller becomes a single synchronous call:

**`audioCache.js` `checkAndCacheComplete`:**
```js
M.maybeStartBulkDownload(trackId);
```
(remove the old `if (...size >= SEGMENTS)` + `downloadAndCacheTrack` block.)

**`ui.js:148-150`** (buffered-range scan, after a segment is marked present):
```js
M.maybeStartBulkDownload(M.currentTrackId);
```

**`audioCache.js:183-187`** (inside `fetchSegment`, after marking the segment):
```js
M.maybeStartBulkDownload(trackId);
```

Because the check-and-claim is now synchronous, even if all three callers fire in the same tick, only the first will start the download.

### Step 3 — Guard the blob-URL swap during playback

`downloadAndCacheTrack` swaps `M.audio.src` to the blob URL mid-playback (`audioCache.js:213-220`). After the refactor, confirm this swap still checks `M.currentTrackId === trackId` and that `wasPlaying` is captured at the moment of the swap (not earlier). Keep the existing `play().catch(()=>{})`.

### Step 4 — (Related, recommended in same pass) Revoke blob URLs

While in this file, address the related memory leak: every `URL.createObjectURL` stored in `M.trackBlobs` is never revoked. See `todo/client-cache-correctness/overview.md` — that plan covers it. If doing both together, the `downloadAndCacheTrack` success path is the natural place to track the URL for later revocation.

## Validation

- **Race reproduction**: this is hard to trigger deterministically. Add a temporary `console.log("[bulk]", trackId)` at the top of the download body and another in `maybeStartBulkDownload` when it returns `true`. Play a track, let it fully buffer, and confirm exactly **one** "started" log and one completion per track — even when forcing the buffered-range scan and `fetchSegment` to both fire (e.g. seek near the end then back).
- **Error path**: temporarily make the download throw (e.g. point the track URL at a 404) and confirm `bulkDownloadStarted` is cleared so a later attempt can retry, rather than permanently preventing caching.
- **Already-cached path**: if a track is already in IndexedDB (`loadTrackBlob` returns a URL on the first line), confirm no full download is triggered and `cachedTracks` is updated.
- **UI**: confirm the buffer bar fills, the cache indicator turns green, and queue/library re-render exactly once per completed cache (not twice).

## Risk / Rollback

- The refactor centralizes three near-duplicate code paths into one — net simplification.
- Risk: if `downloadAndCacheTrack` is called from anywhere else that relied on the internal guard, that caller must be migrated to `maybeStartBulkDownload`. Grep for `downloadAndCacheTrack(` before merging.
- If a download fails after the flag is set and the catch handler fails to clear it, the track becomes uncachable until reload. The `.catch` in `maybeStartBulkDownload` is the safety net — confirm it clears the flag on all rejection paths.
- Rollback = revert `audioCache.js` and the two call-site edits in `ui.js`.