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

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 commit is contained in:
Martin Vogel
2026-10-07 19:20:08 +02:00
parent bf93f0b7c3
commit f9ce38658b
3 changed files with 140 additions and 31 deletions
+68 -31
View File
@@ -990,33 +990,24 @@ static bool build_immutable_uri(const char *path, char *out, size_t out_sz) {
return true;
}
cbm_store_t *cbm_store_open_path_query(const char *db_path) {
if (!db_path) {
return NULL;
}
cbm_store_t *s = calloc(CBM_ALLOC_ONE, sizeof(cbm_store_t));
if (!s) {
return NULL;
}
/* Query tools open the project DB READ-ONLY: a read query must never
* mutate the DB (the previous READWRITE open + WAL write-pragmas did),
* and must work on a read-only DB file / filesystem.
*
* Try a plain READONLY open first — on a normal writable filesystem this
* reads WAL frames correctly via the -shm wal-index. SQLite opens lazily,
* so a read-only-filesystem failure (cannot create -shm for a WAL-mode
* DB) surfaces on first access, not at open time; we probe with a trivial
* read to force it. If the probe fails, retry once with an immutable URI
* that bypasses WAL and reads the main DB file directly.
*
* No SQLITE_OPEN_CREATE on either path — a missing DB must return NULL
* (no ghost .db for unknown/unindexed projects). */
/* Query tools open the project DB READ-ONLY: a read query must never
* mutate the DB (the previous READWRITE open + WAL write-pragmas did),
* and must work on a read-only DB file / filesystem.
*
* Try a plain READONLY open first — on a normal writable filesystem this
* reads WAL frames correctly via the -shm wal-index. SQLite opens lazily,
* so a read-only-filesystem failure (cannot create -shm for a WAL-mode
* DB) surfaces on first access, not at open time; we probe with a trivial
* read to force it. If the probe fails, retry once with an immutable URI
* that bypasses WAL and reads the main DB file directly.
*
* No SQLITE_OPEN_CREATE on either path — a missing DB must return NULL
* (no ghost .db for unknown/unindexed projects). Returns false with s->db
* closed when the DB cannot be opened. */
static bool query_open_first_access(cbm_store_t *s, const char *db_path) {
char open_path[4096];
if (!cbm_path_for_file_api(db_path, open_path, sizeof(open_path))) {
free(s);
return NULL;
return false;
}
int rc = sqlite3_open_v2(open_path, &s->db, SQLITE_OPEN_READONLY, NULL);
if (rc == SQLITE_OK) {
@@ -1036,22 +1027,68 @@ cbm_store_t *cbm_store_open_path_query(const char *db_path) {
* be opened (the read-only-filesystem case). This also keeps the
* common "project not found" path to a single open attempt. */
if (!cbm_file_exists(db_path)) {
free(s);
return NULL;
return false;
}
char uri[ST_QUERY_URI_MAX];
if (!build_immutable_uri(db_path, uri, sizeof(uri))) {
free(s);
return NULL;
return false;
}
rc = sqlite3_open_v2(uri, &s->db, SQLITE_OPEN_READONLY | SQLITE_OPEN_URI, NULL);
if (rc != SQLITE_OK) {
/* sqlite3_open_v2 allocates a handle even on failure — must close it. */
sqlite3_close(s->db);
free(s);
return NULL;
s->db = NULL;
return false;
}
}
return true;
}
#ifdef CBM_ENABLE_TEST_SEAMS
static void (*g_first_access_hook)(void *ctx) = NULL;
static void *g_first_access_hook_ctx = NULL;
void cbm_store_query_first_access_hook_for_testing(void (*hook)(void *ctx), void *ctx) {
g_first_access_hook = hook;
g_first_access_hook_ctx = ctx;
}
static void query_first_access_hook(void) {
if (g_first_access_hook) {
g_first_access_hook(g_first_access_hook_ctx);
}
}
#else
static void query_first_access_hook(void) {}
#endif
cbm_store_t *cbm_store_open_path_query(const char *db_path) {
if (!db_path) {
return NULL;
}
cbm_store_t *s = calloc(CBM_ALLOC_ONE, sizeof(cbm_store_t));
if (!s) {
return NULL;
}
/* One query connection at a time takes its first look at the WAL. On an
* idle DB (no connection open, -shm reset) that first read runs WAL
* recovery, rebuilding the shared wal-index header under SQLite's write
* lock, while another opener reads the same header lock-free before it
* takes its read lock (walTryBeginRead). Across processes SQLite's file
* locks order the two; between this process's request threads nothing
* did, and TSan reported walTryBeginRead against walIndexRecover from
* concurrent index_repository requests. Holding one process-wide mutex
* across open + first access puts a recovery before the next connection's
* first read. The probe is one sqlite_master read, so the hold is short. */
sqlite3_mutex *first_access = sqlite3_mutex_alloc(SQLITE_MUTEX_STATIC_APP1);
sqlite3_mutex_enter(first_access);
bool opened = query_open_first_access(s, db_path);
query_first_access_hook();
sqlite3_mutex_leave(first_access);
if (!opened) {
free(s);
return NULL;
}
s->db_path = heap_strdup(db_path);
+7
View File
@@ -385,6 +385,13 @@ cbm_store_t *cbm_store_open_path_existing(const char *db_path);
* exist — never creates a new .db file. */
cbm_store_t *cbm_store_open_path_query(const char *db_path);
#ifdef CBM_ENABLE_TEST_SEAMS
/* Every following query open calls `hook(ctx)` inside its first-access
* section (after the first read, the process-wide lock still held) until the
* hook is cleared with NULL. Test builds only. */
void cbm_store_query_first_access_hook_for_testing(void (*hook)(void *ctx), void *ctx);
#endif
/* Validate and seal an existing DB for atomic replacement without creating or
* migrating its schema. Returns OK when sealed, NOT_FOUND when the bytes are
* definitely corrupt/incompatible and should be quarantined, or ERR when the
+65
View File
@@ -20,6 +20,8 @@
#include <string.h>
#include <sys/stat.h>
#include <unistd.h>
#include "../src/foundation/compat_thread.h"
#include <stdatomic.h>
static void tsc_cleanup_db(const char *db_path) {
char sidecar[512];
@@ -426,7 +428,70 @@ TEST(remove_db_sidecars_rejects_truncated_suffix_path) {
PASS();
}
/* The first read of a query connection on an idle WAL DB runs WAL recovery,
* rebuilding the shared wal-index header; another connection of this process
* opening at the same moment read that header lock-free (TSan, three CI
* sightings: walTryBeginRead vs walIndexRecover from concurrent
* index_repository requests). A query open's first access is therefore one
* process-wide section, SQLite's SQLITE_MUTEX_STATIC_APP1. With opener A held
* inside it, the section must not be free.
* Windows: SQLite's sqlite3_mutex_try reports BUSY unconditionally without
* SQLITE_WIN32_MUTEX_TRYENTER, so there the assertion cannot go RED; the
* mutex is the same code on every platform and the RED proof is POSIX. */
typedef struct {
const char *path;
atomic_int inside;
atomic_int release;
} tsc_first_access_t;
static void tsc_first_access_hold(void *ctx) {
tsc_first_access_t *p = ctx;
atomic_store_explicit(&p->inside, 1, memory_order_release);
while (!atomic_load_explicit(&p->release, memory_order_acquire)) {}
}
static void *tsc_first_access_open(void *arg) {
tsc_first_access_t *p = arg;
cbm_store_t *s = cbm_store_open_path_query(p->path);
if (s) {
cbm_store_close(s);
}
return NULL;
}
TEST(store_query_first_access_is_one_process_section) {
char *dir = th_mktempdir("cbm_first_access");
ASSERT_NOT_NULL(dir);
char db[512];
snprintf(db, sizeof(db), "%s/g.db", dir);
cbm_store_t *w = cbm_store_open_path(db);
ASSERT_NOT_NULL(w);
cbm_store_close(w);
tsc_first_access_t p = {.path = db};
atomic_init(&p.inside, 0);
atomic_init(&p.release, 0);
cbm_store_query_first_access_hook_for_testing(tsc_first_access_hold, &p);
cbm_thread_t opener;
ASSERT_EQ(cbm_thread_create(&opener, 0, tsc_first_access_open, &p), 0);
while (!atomic_load_explicit(&p.inside, memory_order_acquire)) {}
sqlite3_mutex *section = sqlite3_mutex_alloc(SQLITE_MUTEX_STATIC_APP1);
int try_rc = sqlite3_mutex_try(section);
if (try_rc == SQLITE_OK) {
sqlite3_mutex_leave(section);
}
atomic_store_explicit(&p.release, 1, memory_order_release);
(void)cbm_thread_join(&opener);
cbm_store_query_first_access_hook_for_testing(NULL, NULL);
tsc_cleanup_db(db);
th_rmtree(dir);
ASSERT_EQ(try_rc, SQLITE_BUSY);
PASS();
}
SUITE(store_checkpoint) {
RUN_TEST(store_query_first_access_is_one_process_section);
RUN_TEST(checkpoint_does_not_truncate_wal);
RUN_TEST(seal_for_atomic_publish_makes_main_file_self_contained);
RUN_TEST(seal_for_atomic_publish_fails_closed_while_reader_pins_wal);