Skip to content

Fix prefetch race condition, transaction observable leak, and media store getUrl crash#3396

Merged
olgahaha merged 5 commits intomainfrom
copilot/fix-concurrency-bugs
Apr 16, 2026
Merged

Fix prefetch race condition, transaction observable leak, and media store getUrl crash#3396
olgahaha merged 5 commits intomainfrom
copilot/fix-concurrency-bugs

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 2, 2026

Three concurrency and error-handling bugs in core shared services causing non-deterministic progress reporting, potential observable memory leaks, and silent media display failures.

Changes

  • Prefetch race condition (dia-backend-asset-prefetching.service.ts): Replace Promise.all + concurrent .map with a sequential for...of loop. Concurrent async callbacks were racing on the shared currentCount variable, causing skipped or out-of-order progress callbacks.

    // Before – concurrent writes to currentCount
    await Promise.all(diaBackendAssets.map(async asset => {
      await this.downloadingService.storeRemoteCapture(...);
      currentCount += 1; // race condition
      onStored(currentCount, totalCount);
    }));
    
    // After – sequential, deterministic
    for (const diaBackendAsset of diaBackendAssets) {
      await this.downloadingService.storeRemoteCapture(...);
      currentCount += 1;
      onStored(currentCount, totalCount);
    }
  • Transaction observable leak (dia-backend-transaction-repository.service.ts): Add catchError returning EMPTY at the end of the downloadExpired$ pipeline. A failure anywhere in the concatMap chain previously terminated the observable uncleanly, leaving subscribers hanging. Imports EMPTY and catchError from rxjs.

  • Media store getUrl crash (media-store.service.ts): Wrap getUrl() body in try/catch. Failures from Capacitor.convertFileSrc(), getUri(), or readWithFileSystem() were propagating as unhandled rejections and silently breaking the display pipeline. Now logs the error and returns '' as a graceful fallback.

…d media store crash

- Replace Promise.all with sequential for...of in DiaBackendAssetPrefetchingService.prefetch()
  to eliminate concurrent writes to the shared currentCount variable
- Add catchError(EMPTY) to the downloadExpired$ observable chain in
  DiaBackendTransactionRepository to prevent hanging subscriptions on failure
- Wrap MediaStore.getUrl() in try-catch to return empty string gracefully
  instead of silently crashing the display pipeline

Agent-Logs-Url: https://github.com/numbersprotocol/capture-cam/sessions/ff0c4f0b-d6a2-43f0-8f11-98535fdf9f3f

Co-authored-by: numbers-official <181934381+numbers-official@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix concurrency bugs in prefetching service Fix prefetch race condition, transaction observable leak, and media store getUrl crash Apr 2, 2026
Copilot AI requested a review from numbers-official April 2, 2026 21:48
Omni and others added 3 commits April 16, 2026 06:15
Replace fully sequential for...of with batched Promise.all (batch size 5).
Avoids the original 100-concurrent-request race condition while preventing
the ~10x slowdown of fully sequential processing.

100 assets: ~4-8s (batched) vs ~20s (sequential) vs ~2-5s (unbounded)
@olgahaha olgahaha marked this pull request as ready for review April 16, 2026 06:29
@olgahaha olgahaha merged commit ce164b1 into main Apr 16, 2026
9 checks passed
@olgahaha olgahaha deleted the copilot/fix-concurrency-bugs branch April 16, 2026 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature][High] Fix concurrency bugs: prefetch race condition, transaction observable leak, media store crash

3 participants