Skip to content

fix(store): one process-wide section for a query connection's first access - #2561

Open
DeusData wants to merge 1 commit into
mainfrom
fix/sqlite-wal-query-open-race
Open

DeusData wants to merge 1 commit into
mainfrom
fix/sqlite-wal-query-open-race

Conversation

@DeusData

@DeusData DeusData commented Oct 7, 2026

Copy link
Copy Markdown
Owner

What

cbm_store_open_path_query now opens and probes the database inside one process-wide section: SQLite's SQLITE_MUTEX_STATIC_APP1, the mutex SQLite reserves for the application. The section covers the read-only open, its first read and the immutable fallback. The queries that follow run outside it, as before.

Why

TSan reported the same data race three times in two days: on #2458's test-tsan (ubuntu-24.04-arm) leg on 2026-10-06, and twice on #2348's test-tsan (macos-14) leg on 2026-10-07, the second time on the re-run. The two sides were:

  • walTryBeginRead (sqlite3.c:69884);
  • walIndexReadHdr → walIndexRecover (:69480).

Both threads were concurrent index_repository requests in daemon_application, each reading the project record through its own cbm_store_open_path_query connection.

On an idle WAL database (no connection open, -shm reset), the first read of a new connection runs WAL recovery. Recovery rebuilds the shared wal-index header under SQLite's write lock. A second connection in the same process, opening at that moment, reads that header without a lock at the start of walTryBeginRead, before it takes its read lock. Across processes, SQLite's file locks order the two. Between the daemon's request threads, nothing did. This makes #2348's required TSan leg red, although #2348 does not touch this code.

Proof

Step Result
RED: local stress test under test-runner-tsan (6 threads open one idle WAL DB per round, -shm removed between rounds, 60 rounds) aborted on the CI pair walTryBeginRead :69884 vs walIndexReadHdr :69480
GREEN: same stress test with the fix, 3 runs 0 TSan reports, all passed
Regression test store_query_first_access_is_one_process_section without the lock FAIL: try_rc == 0, expected SQLITE_BUSY == 5
Same test with the lock PASS
store_checkpoint + daemon_application under test-runner-tsan on the final tree 76 passed, 0 TSan reports

The stress test was only for reproduction and is not committed, because its verdict depends on timing. The committed test is deterministic:

  • A test seam (cbm_store_query_first_access_hook_for_testing, test builds only) holds one opener inside the section.
  • While it is held, sqlite3_mutex_try on the section's mutex must report SQLITE_BUSY.
  • On Windows, SQLite's sqlite3_mutex_try reports BUSY unconditionally without SQLITE_WIN32_MUTEX_TRYENTER. So the failing case is shown on POSIX. The locking code is the same on every platform.

Cost

The section holds one sqlite_master read per query open (plus WAL recovery, once, on an idle database). Opens of different projects now take turns for that read; everything after the open is unchanged.

…ccess

On an idle WAL database (no connection open, -shm reset) the first read of a
new connection runs WAL recovery: walIndexRecover rebuilds the shared
wal-index header under SQLite's write lock. A second connection of the same
process opening at that moment reads that header lock-free at the start of
walTryBeginRead, before it takes its read lock. Across processes SQLite's
file locks order the two; between the daemon's request threads nothing did.
TSan reported exactly that pair three times in CI (2026-10-06 on #2458's
tsan-arm leg, 2026-10-07 twice on #2348's macOS leg), from concurrent
index_repository requests reading the project record through
cbm_store_open_path_query.

cbm_store_open_path_query now opens and probes the database (the plain
read-only open, its first read, and the immutable fallback) inside
SQLITE_MUTEX_STATIC_APP1, the process-wide mutex SQLite reserves for the
application. The probe is one sqlite_master read, so the section is short;
the queries that follow run outside it, as before.

Regression test store_query_first_access_is_one_process_section: a test
seam holds one opener inside the section, and sqlite3_mutex_try on the
section's mutex must then report SQLITE_BUSY. It fails without the lock
(try returns SQLITE_OK) and passes with it. SQLite's Windows build reports
BUSY from sqlite3_mutex_try unconditionally, so the failing case is shown on
POSIX; the locking code is the same on every platform.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>

This branch has not been deployed

No deployments
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