Skip to content

fix(common): stop expireCache looping with no archived attachments - #1092

Open
giaBaoJS wants to merge 1 commit into
powersync-ja:mainfrom
giaBaoJS:fix/attachments-expire-cache-loop
Open

fix(common): stop expireCache looping with no archived attachments#1092
giaBaoJS wants to merge 1 commit into
powersync-ja:mainfrom
giaBaoJS:fix/attachments-expire-cache-loop

Conversation

@giaBaoJS

@giaBaoJS giaBaoJS commented Sep 6, 2026

Copy link
Copy Markdown

The bug

AttachmentQueue.expireCache() never resolves when the number of archived attachments is at or below archivedCacheLimit, which is the normal case. It keeps re-running the same SELECT against the attachments table forever.

Reproduction

const queue = new AttachmentQueue({ db, localStorage, remoteStorage, watchAttachments });
// No archived attachments (or fewer than archivedCacheLimit, which defaults to 100).
await queue.expireCache(); // never resolves; the same query runs in a tight loop

Root cause

AttachmentContext.deleteArchivedAttachments returns a "cleanup finished" flag: the last line returns archivedAttachments.length < limit, so a partial page means there is nothing left to page through. The early return for an empty page returned false instead, which is the opposite: an empty page is the smallest possible partial page. AttachmentQueue.expireCache loops while (!isDone) on that flag, so the empty case spins forever.

The other caller, AttachmentQueue.syncStorage, ignores the return value, which is why this has not shown up there.

For comparison, the same operation in the older @powersync/attachments package (AbstractAttachmentQueue.expireCache) returns immediately when the query comes back empty.

What the tests assert

  • AttachmentContext > deleteArchivedAttachments > reports completion when there is nothing to delete: deleteArchivedAttachments() returns true for an empty page, and neither the callback nor the delete statement runs. Without the fix this fails with expected false to be true.
  • AttachmentContext > deleteArchivedAttachments > reports completion after deleting a partial page: pins the existing behaviour that true means "done" for a page smaller than the internal limit.
  • AttachmentQueue > expireCache > stops once there is nothing archived left to delete: expireCache() resolves after a single query. The fake database throws after a few queries so the non-terminating case fails as an assertion rather than hanging the suite.

pnpm --filter @powersync/common test: 27 tests passing before, 30 after.

@changeset-bot

changeset-bot Bot commented Sep 6, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5642822

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@powersync/common Patch
@powersync/node Patch
@powersync/react-native Patch
@powersync/tanstack-react-query Patch
@powersync/web Patch
@powersync/diagnostics-app Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

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.

1 participant