mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-10-16 05:10:08 +08:00
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:
+68
-31
@@ -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);
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user