Skip to content

Fine-tune SQLite cache handling#130

Open
jsandfordhughescoop wants to merge 1 commit intocalebporzio:mainfrom
jsandfordhughescoop:race-condition
Open

Fine-tune SQLite cache handling#130
jsandfordhughescoop wants to merge 1 commit intocalebporzio:mainfrom
jsandfordhughescoop:race-condition

Conversation

@jsandfordhughescoop
Copy link

Summary

This PR fixes a SQLite cache initialisation race in Sushi that could cause SQLSTATE[HY000]: ... no such table when multiple processes boot the same model against a fresh cache file.

Root cause

Sushi previously treated a cache file as valid based on existence + mtime only. During rebuild, it created/truncated the final cache file before running migrations. A concurrent process could then see that file as “fresh” and query it before the table existed.

What changed

  • Hardened cache-hit check to require a non-empty cache file (validate empty cache file by filesize #124):
    • file_exists($cachePath) && filesize($cachePath) > 0 && filemtime($dataPath) <= filemtime($cachePath)
  • Made cache rebuild atomic:
    • Build into a temp SQLite file in the same directory.
    • Run migrate() against the temp file.
    • Set temp mtime from the reference file.
    • rename() temp file to final cache path.
  • Added regression test:
    • test_rebuilds_empty_cache_file_even_when_its_timestamp_is_fresh
  • Updated the existing concurrent-creation test mock to allow the additional connection call introduced by temp->final swap.

Why this fixes the error

The final cache path is no longer exposed in an empty/partially-initialised state. Only a fully migrated SQLite file is published via atomic rename, and zero-byte files are never treated as valid cache hits.

Backwards compatibility

No public API changes.
Existing in-memory fallback behaviour is unchanged.

@jsandfordhughescoop jsandfordhughescoop marked this pull request as ready for review February 26, 2026 07: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.

1 participant