7.7 KiB

Raw Permalink Blame History

Client Cache Correctness: Revoke Blob URLs, LRU Pruning, Real `streamOnly`

Priority: Medium · Effort: Medium · Risk: Low

Problem

Three independent correctness/efficiency bugs in the client caching layer:

Blob URL leak. URL.createObjectURL(blob) results stored in M.trackBlobs (public/audioCache.js:80,127) are never revoked. M.trackBlobs.clear() (queue.js:188) drops the references but the URLs stay live in the document until unload — a slow memory leak over long sessions with many cached tracks.
pruneCache is expensive and non-LRU. public/audioCache.js:27-54 calls TrackStorage.get(key) for every cached track (pulling each blob out of IndexedDB) just to read .size, and evicts oldest-first by Map iteration order — not by the cachedAt timestamp that is actually written at public/trackStorage.js:86. So eviction is arbitrary, not least-recently-used.
streamOnly doesn't do what it says. The flag is stored (audioCache.js:8-15) and checked by exactly one context-menu item (trackContainer.js:752), but the background prefetch loop (audioCache.js:226) never consults it — so "stream-only mode" still fetches every segment in the background and still triggers bulk caching.

Affected Locations

public/audioCache.js:27-54 — pruneCache
public/audioCache.js:80,127 — blob URL creation
public/audioCache.js:224-262 — prefetch loop (no streamOnly check)
public/trackStorage.js:139-155 — getStats (loads all blobs)
public/trackStorage.js:86 — cachedAt is written but unused
public/queue.js:188 — M.trackBlobs.clear() without revoke

Implementation Plan

Part A — Track and revoke blob URLs

Step A1: Track blob URLs in a single Map with a revocation helper.

M.trackBlobs is already Map<trackId, blobUrl>. Add:

// public/audioCache.js
M.revokeTrackBlob = function (trackId) {
  const url = M.trackBlobs.get(trackId);
  if (url) {
    URL.revokeObjectURL(url);
    M.trackBlobs.delete(trackId);
  }
};

Step A2: Revoke on eviction and on explicit clear.

In pruneCache (after Part B), when a track is evicted from IndexedDB, also call M.revokeTrackBlob(trackId) so the in-memory URL is released.
In queue.js:188 (clearAllCaches), revoke every entry before clearing:

for (const url of M.trackBlobs.values()) URL.revokeObjectURL(url);
M.trackBlobs.clear();

When a track is removed from the library (track_removed handler in channelSync.js), revoke its blob if present (it can't be played anymore).

Step A3: Don't double-revoke. URL.revokeObjectURL is safe to call on an already-revoked URL, but guard with the Map check anyway to keep state clean.

Part B — Make pruning LRU and cheap

Step B1: Store size at write time, avoid loading blobs at prune time.

Extend the IndexedDB record to carry size (the blob already has .size — store it alongside). In trackStorage.js:

// in set(): store { filename (keyPath), blob, size, cachedAt }
const record = { filename: trackId, blob, size: blob.size, cachedAt: Date.now() };

Add a lightweight metadata accessor that uses a cursor and reads only size/cachedAt, not the blob:

// trackStorage.js
getMetaList: function () {
  return new Promise((resolve) => {
    const result = [];
    const tx = db.transaction(STORE, "readonly");
    const store = tx.objectStore(STORE);
    const req = store.openCursor();
    req.onsuccess = (e) => {
      const cursor = e.target.result;
      if (cursor) {
        const v = cursor.value;
        result.push({ id: v.filename, size: v.size || 0, cachedAt: v.cachedAt || 0 });
        cursor.continue();
      } else {
        resolve(result);
      }
    };
    req.onerror = () => resolve([]);
  });
}

Step B2: Rewrite pruneCache to be LRU and blob-free.

// audioCache.js
async function pruneCache() {
  const limit = M.cacheLimitBytes ?? 500 * 1024 * 1024;   // tune as needed
  const meta = await TrackStorage.getMetaList();
  let total = meta.reduce((s, m) => s + m.size, 0);
  if (total <= limit) return;

  // Evict least-recently-used first.
  meta.sort((a, b) => a.cachedAt - b.cachedAt);
  for (const m of meta) {
    if (total <= limit) break;
    await TrackStorage.delete(m.id);
    M.revokeTrackBlob(m.id);
    M.cachedTracks.delete(m.id);
    total -= m.size;
  }
}

Step B3: Update cachedAt on access (optional, makes LRU reflect actual use). When a cached track is played (loadTrackBlob returns a URL), bump its cachedAt. If skipped, the policy becomes "first-in-first-out", which is still better than today's arbitrary order.

Step B4: Rewrite getStats to use getMetaList so the settings/stats panel doesn't load every blob either.

Part C — Make `streamOnly` actually disable background fetching

Step C1: Consult the flag in the prefetch loop.

In audioCache.js:226 (top of the prefetch loop):

M.prefetchSegments = async function () {
  if (prefetching) return;
  if (M.streamOnly) return;            // <-- new
  prefetching = true;
  try {
    // ... existing logic ...
  } finally {
    prefetching = false;
  }
};

Step C2: Also short-circuit the buffered-range-scan-triggered bulk download when in stream-only mode, since streaming clients don't want IndexedDB writes. Easiest: have maybeStartBulkDownload (see todo/duplicate-download-race/overview.md) check M.streamOnly and return false.

Step C3: Decide intent for the already-playing track. Even in stream-only mode the current track is being buffered by the browser's own media element — that's fine and desirable. The flag is about suppressing additional segment prefetching ahead of the playhead and bulk caching. Confirm the UX label reflects this ("Don't pre-cache ahead" rather than "stream nothing").

Validation

Blob revoke: open DevTools → Memory, take a snapshot; play and fully cache ~10 tracks; confirm trackBlobs grows by 10 and the document's blob URL count grows accordingly. Trigger a prune (or set a tiny cacheLimitBytes) and confirm the count drops as tracks are evicted. Run clearAllCaches() and confirm all blob URLs are revoked.
LRU: with a small cache limit, cache tracks A, B, C; play A again (bump cachedAt if Part B3 is implemented); cache D. Confirm B (not A) is evicted.
Prune performance: with a large cache (e.g. 200 tracks), instrument pruneCache and confirm it no longer reads blob payloads — wall time should drop from "loads every blob" to a cheap cursor scan.
streamOnly: toggle the flag on, play a track, seek around, and confirm via DevTools Network that only the media element's own range requests fire (no /api/tracks/:id segment prefetches), and IndexedDB write count stays flat. Toggle off and confirm prefetching resumes.
Regression: with streamOnly off, confirm normal caching behavior (green indicators, blob swap, IndexedDB entries) is unchanged.

Risk / Rollback

IndexedDB schema change (adding size/cachedAt fields to the record): old records written before this change will lack those fields. getMetaList must default missing fields to 0 (shown above), and pruneCache must handle size === 0 gracefully (skip eviction of unknown-size entries rather than evicting everything). Consider a one-time migration that deletes records lacking size, or simply let them age out.
Revoking a blob URL that is currently the src of the audio element will break playback. Only revoke blobs for tracks that are not the currently-playing track, or that have just been removed from the library. Add a guard: if (trackId === M.currentTrackId) return; in revokeTrackBlob callers that run during playback.
streamOnly is additive; default stays off.
Rollback per part is independent — A, B, C can land separately.

7.7 KiB Raw Permalink Blame History

Client Cache Correctness: Revoke Blob URLs, LRU Pruning, Real streamOnly