Skip to content

fix: serialize cold eval/task cache init across processes - #3156

Open
Arshgill01 wants to merge 1 commit into
cachix:mainfrom
Arshgill01:cursor/fix-cold-concurrent-eval-cache-3018
Open

Arshgill01 wants to merge 1 commit into
cachix:mainfrom
Arshgill01:cursor/fix-cold-concurrent-eval-cache-3018

Conversation

@Arshgill01

Copy link
Copy Markdown
Contributor

Fixes #3133.

Concurrent devenv shell entries into a project that does not yet have .devenv/ raced on eval-cache (and task-cache) SQLite initialization. One process created the database; another failed with Failed to initialize eval cache database: Database error: (code: 5) database is locked. Under more contention the same window also produced task-cache init failures, SQLITE_IOERR_DELETE_NOENT (code 5898) after a loser deleted the DB, and Failed to remove existing GC root: No such file or directory. Warm .devenv/ was already fine.

busy_timeout(10s) and WAL with DELETE fallback on SQLITE_IOERR_SHMMAP were not enough: sqlite3_open(CREATE) / PRAGMA journal_mode=WAL can return SQLITE_BUSY before the timeout is installed, and a migration error used to delete the database out from under the process that held it.

Changes

  • Take an exclusive flock on a sibling of the SQLite file (*.db.init.lock, not the .db itself) around create + migrate in Database::new, so eval cache and task cache share the same cold-init serialization.
  • Hold that lock on a dedicated OS thread so waiters do not stall the tokio runtime.
  • Never delete the database on a busy/locked migration error; retry instead.
  • Treat a missing GC-root symlink as success when replacing it, so two shells creating the same root no longer lose the TOCTOU race.
  • Regression tests: concurrent cold/warm Database::new, concurrent cold TaskCache::new, and tests/cold-concurrent-shell (two concurrent devenv shell -q -- true against a missing .devenv/, then the same against a warm one).

Test plan

  • cargo test -p devenv-cache-core (includes concurrent_cold_open_succeeds / concurrent_warm_open_succeeds)
  • cargo test -p devenv-tasks --lib -- test_cold_cache_allows_concurrent_open test_warm_cache_allows_concurrent_open
  • cargo clippy -p devenv-cache-core --all-targets -- -D warnings
  • devenv-run-tests run tests --only cold-concurrent-shell (needs a full devenv/Nix environment; not run in this agent VM)

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

🔍 Suggested Reviewers

Based on git blame analysis of the changed lines, the following contributors have significant experience with the modified code:

  • @sandydoo - 100.0% of changed lines (9 lines)

Please consider reviewing this PR as you have authored significant portions of the code being modified. Your expertise would be valuable! 🙏

This comment was automatically generated by git-blame-auto-reviewer

Last updated: 2026-09-22T03:49:29.191Z

@cursor
cursor Bot force-pushed the cursor/fix-cold-concurrent-eval-cache-3018 branch 5 times, most recently from ed8bfab to e543fb9 Compare September 8, 2026 03:49
@Arshgill01

Copy link
Copy Markdown
Contributor Author

Rebased onto latest main (behind_by: 0, mergeable). Kept the #3133 changelog entry with main’s newer 2.3.0/2.3.1 notes. Preserved flock around Database::new and GC-root NotFound-as-success on ensure_gc_root.

@cursor
cursor Bot force-pushed the cursor/fix-cold-concurrent-eval-cache-3018 branch 2 times, most recently from 2518db0 to 7239326 Compare September 14, 2026 04:35
@Arshgill01

Copy link
Copy Markdown
Contributor Author

Rebased onto latest main (behind_by: 0, mergeable). Kept the #3133 changelog entry with main’s newer 2.3.1/2.3.2 notes. Preserved flock around Database::new and GC-root NotFound-as-success on ensure_gc_root.

@cursor
cursor Bot force-pushed the cursor/fix-cold-concurrent-eval-cache-3018 branch 2 times, most recently from 5d3724c to 78f5a90 Compare September 18, 2026 03:48
@cursor
cursor Bot force-pushed the cursor/fix-cold-concurrent-eval-cache-3018 branch from 78f5a90 to bfab7a4 Compare September 21, 2026 03:39
Concurrent `devenv shell` entries into a project without `.devenv/` raced
on SQLite create+migrate. One process succeeded; another failed with
`database is locked` (and sometimes disk I/O 5898 after the loser deleted
the database). Warm `.devenv/` was fine.

Hold an exclusive flock around Database::new, never delete the DB on a
busy/locked migration error, and treat concurrent GC-root removal as
success when the symlink is already gone.

Fixes cachix#3133

Co-authored-by: Arshdeep singh <arshgill6120@gmail.com>
@cursor
cursor Bot force-pushed the cursor/fix-cold-concurrent-eval-cache-3018 branch from bfab7a4 to 9b7a402 Compare September 22, 2026 03:49
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.

Concurrent devenv shell calls result in "Failed to initialize eval cache database: database is locked"

2 participants