Fix SQLite writer atomic cache publish

internal/cbm/sqlite_writer.c

@@ -27,6 +27,16 @@
 #include <stdint.h>
 #include <stdbool.h>
 
+#ifdef _WIN32
+#include <io.h>
+#include <process.h>
+#include <windows.h>
+#define cbm_writer_getpid _getpid
+#else
+#include <unistd.h>
+#define cbm_writer_getpid getpid
+#endif
+
 #define CBM_PAGE_SIZE 65536
 
 /* SQLite reserves the page containing the 1 GiB file offset (the "pending byte"
@@ -1707,6 +1717,8 @@ static uint32_t build_node_index_sorted(FILE *fp, uint32_t *next_page, CBMDumpNo
 typedef struct {
     FILE *fp;
     uint32_t next_page;
+    char final_path[CBM_SZ_4K];
+    char temp_path[CBM_SZ_4K];
     const char *project;
     const char *root_path;
     const char *indexed_at;
@@ -1720,6 +1732,60 @@ typedef struct {
     int token_vec_count;
 } write_db_ctx_t;
 
+static int make_writer_temp_path(const char *path, const void *token, char *out, size_t out_size) {
+    int n = snprintf(out, out_size, "%s.tmp.%ld.%p", path, (long)cbm_writer_getpid(), token);
+    return (n >= 0 && (size_t)n < out_size) ? 0 : ERR_WRITE_FAILED;
+}
+
+static int sync_writer_output(FILE *fp) {
+    if (fflush(fp) != 0) {
+        return ERR_WRITE_FAILED;
+    }
+#ifdef _WIN32
+    return _commit(_fileno(fp)) == 0 ? 0 : ERR_WRITE_FAILED;
+#else
+    return fsync(fileno(fp)) == 0 ? 0 : ERR_WRITE_FAILED;
+#endif
+}
+
+static int discard_writer_output(write_db_ctx_t *w, int rc) {
+    if (w->fp) {
+        (void)fclose(w->fp);
+        w->fp = NULL;
+    }
+    if (w->temp_path[0]) {
+        (void)cbm_unlink(w->temp_path);
+    }
+    return rc;
+}
+
+static int publish_writer_output(write_db_ctx_t *w) {
+    if (sync_writer_output(w->fp) != 0) {
+        return discard_writer_output(w, ERR_WRITE_FAILED);
+    }
+    if (fclose(w->fp) != 0) {
+        w->fp = NULL;
+        if (w->temp_path[0]) {
+            (void)cbm_unlink(w->temp_path);
+        }
+        return ERR_WRITE_FAILED;
+    }
+    w->fp = NULL;
+    if (!w->temp_path[0] || !w->final_path[0]) {
+        return 0;
+    }
+    if (cbm_rename_replace(w->temp_path, w->final_path) != 0) {
+        (void)cbm_unlink(w->temp_path);
+        return ERR_WRITE_FAILED;
+    }
+    /* Sidecars are removed only after the replacement succeeds. On POSIX,
+     * readers of the old generation retain their unlinked handles. On
+     * Windows, an incompatible open handle makes MoveFileExW fail before
+     * this cleanup, preserving the old DB and its sidecars. */
+    cbm_remove_db_sidecars(w->final_path);
+    return 0;
+}
+
 /* Callback type for building a record from an item at index i. */
 typedef uint8_t *(*build_record_fn)(const void *items, int i, int *out_len);
 typedef int64_t (*get_rowid_fn)(const void *items, int i);
@@ -1992,20 +2058,17 @@ static int write_db_after_nodes(write_db_ctx_t *w, uint32_t nodes_root) {
     int rc =
         write_one_table(w, &edges_root, w->edges, w->edge_count, adapt_build_edge, adapt_edge_id);
     if (rc != 0) {
-        (void)fclose(fp);
-        return rc;
+        return discard_writer_output(w, rc);
     }
     rc = write_one_table(w, &vectors_root, w->vectors, w->vector_count, adapt_build_vector,
                          adapt_vector_id);
     if (rc != 0) {
-        (void)fclose(fp);
-        return rc;
+        return discard_writer_output(w, rc);
     }
     rc = write_one_table(w, &token_vecs_root, w->token_vecs, w->token_vec_count,
                          adapt_build_token_vec, adapt_token_vec_id);
     if (rc != 0) {
-        (void)fclose(fp);
-        return rc;
+        return discard_writer_output(w, rc);
     }
     CBM_PROF_END_N("write_db", "1_data_tables", t_data, node_count + edge_count);
 
@@ -2058,8 +2121,7 @@ static int write_db_after_nodes(write_db_ctx_t *w, uint32_t nodes_root) {
                                  &idx_nodes_name_root, &idx_nodes_file_root, &autoindex_nodes_root);
     CBM_PROF_END_N("write_db", "4_node_indexes_seq", t_node_idx, node_count * NODE_SORT_THREADS);
     if (nrc != 0) {
-        (void)fclose(fp);
-        return nrc;
+        return discard_writer_output(w, nrc);
     }
 
     CBM_PROF_START(t_edge_idx);
@@ -2076,8 +2138,7 @@ static int write_db_after_nodes(write_db_ctx_t *w, uint32_t nodes_root) {
                                  &idx_edges_url_path_root, &autoindex_edges_root);
     CBM_PROF_END_N("write_db", "5_edge_indexes_seq", t_edge_idx, edge_count * EDGE_SORT_THREADS);
     if (erc != 0) {
-        (void)fclose(fp);
-        return erc;
+        return discard_writer_output(w, erc);
     }
 
     // Autoindex for projects(name TEXT PK) — single text column
@@ -2188,12 +2249,10 @@ static int write_db_after_nodes(write_db_ctx_t *w, uint32_t nodes_root) {
     int master_count = sizeof(master) / sizeof(master[0]);
     int rc2 = write_master_page1(fp, master, master_count, next_page);
     if (rc2 != 0) {
-        (void)fclose(fp);
-        return rc2;
+        return discard_writer_output(w, rc2);
     }
     pad_file_to_page_boundary(fp, next_page);
-    (void)fclose(fp);
-    return 0;
+    return publish_writer_output(w);
 }
 
 // --- Streaming writer (incremental bulk node-table append) ---
@@ -2207,21 +2266,20 @@ struct cbm_db_writer {
 };
 
 cbm_db_writer_t *cbm_writer_open(const char *path) {
-    /* Installing a fresh DB generation: drop the destination's leftover
-     * -wal/-shm or a crashed session's WAL gets replayed on top of the
-     * new file at the next open (#897). */
-    cbm_remove_db_sidecars(path);
-    /* cbm_fopen, not raw fopen: the cache dir lives under the user profile,
-     * and an ANSI-CP fopen fails to create the DB on non-ASCII Windows
-     * profiles — the reported phase=dump failure (#996). Everything around
-     * this call (cbm_mkdir_p, sqlite3_open_v2 reopen) is already wide-safe. */
-    FILE *fp = cbm_fopen(path, "wb");
-    if (!fp) {
-        return NULL;
-    }
     cbm_db_writer_t *w = (cbm_db_writer_t *)calloc(CBM_ALLOC_ONE, sizeof(*w));
     if (!w) {
-        (void)fclose(fp);
+        return NULL;
+    }
+    int n = snprintf(w->wc.final_path, sizeof(w->wc.final_path), "%s", path);
+    if (n < 0 || (size_t)n >= sizeof(w->wc.final_path) ||
+        make_writer_temp_path(path, w, w->wc.temp_path, sizeof(w->wc.temp_path)) != 0) {
+        free(w);
+        return NULL;
+    }
+    FILE *fp = cbm_fopen(w->wc.temp_path, "wb");
+    if (!fp) {
+        (void)cbm_unlink(w->wc.temp_path);
+        free(w);
         return NULL;
     }
     w->wc.fp = fp;
@@ -2287,8 +2345,7 @@ int cbm_writer_finalize(cbm_db_writer_t *w, const char *project, const char *roo
     write_db_ctx_t wc = w->wc; /* value copy survives free(w) */
     free(w);
     if (err != 0) {
-        (void)fclose(wc.fp); /* wc is a value copy, valid after free(w) */
-        return err;
+        return discard_writer_output(&wc, err);
     }
     return write_db_after_nodes(&wc, nodes_root);
 }

src/foundation/compat_fs.c

@@ -761,17 +761,19 @@ int cbm_canonical_path(const char *path, char *out, size_t out_sz) {
 
 /* rename() with overwrite semantics on every platform: POSIX rename already
  * replaces atomically; Windows rename fails with EEXIST when the target
- * exists, so use MoveFileExW(MOVEFILE_REPLACE_EXISTING) there (wide paths —
- * raw MoveFileExA would re-mangle non-ASCII cache paths). */
+ * exists, so use write-through MoveFileExW(MOVEFILE_REPLACE_EXISTING) there
+ * (wide paths — raw MoveFileExA would re-mangle non-ASCII cache paths). */
 int cbm_rename_replace(const char *src, const char *dst) {
 #ifdef _WIN32
     wchar_t *wsrc = cbm_utf8_to_wide(src);
     wchar_t *wdst = cbm_utf8_to_wide(dst);
     int ret = CBM_NOT_FOUND;
     if (wsrc && wdst) {
-        ret = MoveFileExW(wsrc, wdst, MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED)
-                  ? 0
-                  : CBM_NOT_FOUND;
+        ret =
+            MoveFileExW(wsrc, wdst,
+                        MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED | MOVEFILE_WRITE_THROUGH)
+                ? 0
+                : CBM_NOT_FOUND;
     }
     free(wsrc);
     free(wdst);
@@ -782,11 +784,10 @@ int cbm_rename_replace(const char *src, const char *dst) {
 }
 
 /* Remove a SQLite database's -wal/-shm sidecars (both platforms). Any code
- * path that installs a FRESH database file at a path where a previous
- * generation lived must call this first: SQLite decides whether to replay a
- * WAL purely from the sidecar's own header/checksums, so a leftover WAL
- * from a crashed session is recovered ON TOP of the freshly installed file
- * at the next open, splicing old-generation pages into it (#897). */
+ * path that installs a FRESH database file where a previous generation lived
+ * must remove them before the new generation can be opened: SQLite decides
+ * whether to replay a WAL purely from the sidecar's own header/checksums, so
+ * a leftover WAL can splice old-generation pages into the new file (#897). */
 void cbm_remove_db_sidecars(const char *db_path) {
     if (!db_path || !db_path[0]) {
         return;

src/foundation/compat_fs.h

@@ -46,12 +46,12 @@ bool cbm_mkdir_p(const char *path, int mode);
 
 /* Delete a file. Returns 0 on success. */
 int cbm_unlink(const char *path);
-/* Remove <db_path>-wal/-shm. MUST be called by any path installing a fresh
- * DB file where a previous generation lived — a leftover WAL is otherwise
- * replayed on top of the new file at the next open (#897). */
+/* Remove <db_path>-wal/-shm. Any path installing a fresh DB generation must
+ * do this before the new generation can be opened; a leftover WAL is
+ * otherwise replayed on top of the new file (#897). */
 void cbm_remove_db_sidecars(const char *db_path);
 /* rename() that replaces an existing destination on every platform
- * (Windows rename fails with EEXIST; this uses MoveFileExW there). */
+ * (Windows rename fails with EEXIST; this uses write-through MoveFileExW). */
 int cbm_rename_replace(const char *src, const char *dst);
 /* Canonicalize an EXISTING path (realpath / wide GetFullPathNameW). Locale-
  * independent on Windows — never routes UTF-8 through the ANSI CRT (#973).