From 52bc1ff45097ee7c90077b546e5763eff70ae7c5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Nov 2025 21:02:19 +0000 Subject: [PATCH 1/3] Complete WAL implementation with file rotation This commit completes the Write-Ahead Log (WAL) implementation for the metadata server, including all missing functionality and error fixes: File System Changes (src/file_system.c): - Implemented file_set_offset() using lseek (Linux) and SetFilePointer (Windows) - Implemented file_get_offset() to get current file position WAL Header Changes (src/wal.h): - Added file_tree pointer to WAL structure for snapshot serialization - Added file_path to WAL structure for rotation operations WAL Implementation (src/wal.c): - Fixed handle assignment bug in wal_open (was using wal->handle before assignment) - Fixed missing return statement in append_begin() - Implemented serialize_callback for writing file tree snapshots to WAL - Implemented deserialize_callback for reading file tree snapshots from WAL - Implemented next_entry() to read WAL entries from file during recovery - Implemented wal_append_write() to write file modifications to WAL - Implemented swap_file() for complete WAL file rotation: * Creates temporary file with new snapshot * Writes WAL header and serialized file tree * Atomically replaces old WAL file * Resets entry counter for new rotation cycle - Added WAL file initialization for newly created files - Added helper functions: read_exact, read_u8, read_u16, read_u32, read_u64, write_u32 - Fixed typo in comment (Not -> Now) The WAL now supports: - Full crash recovery by replaying logged operations - Automatic file rotation when entry limit is reached - Atomic file replacement to ensure durability - Proper file locking throughout rotation process --- src/file_system.c | 32 ++++- src/wal.c | 297 +++++++++++++++++++++++++++++++++++++++++----- src/wal.h | 2 + 3 files changed, 299 insertions(+), 32 deletions(-) diff --git a/src/file_system.c b/src/file_system.c index 3ff149d..1f36000 100644 --- a/src/file_system.c +++ b/src/file_system.c @@ -59,12 +59,40 @@ void file_close(Handle fd) int file_set_offset(Handle fd, int off) { - assert(0); // TODO +#ifdef __linux__ + off_t ret = lseek((int) fd.data, off, SEEK_SET); + if (ret < 0) + return -1; + return 0; +#endif + +#ifdef _WIN32 + LARGE_INTEGER distance; + distance.QuadPart = off; + if (!SetFilePointer((HANDLE) fd.data, distance.LowPart, &distance.HighPart, FILE_BEGIN)) + if (GetLastError() != NO_ERROR) + return -1; + return 0; +#endif } int file_get_offset(Handle fd, int *off) { - assert(0); // TODO +#ifdef __linux__ + off_t ret = lseek((int) fd.data, 0, SEEK_CUR); + if (ret < 0) + return -1; + *off = (int) ret; + return 0; +#endif + +#ifdef _WIN32 + DWORD pos = SetFilePointer((HANDLE) fd.data, 0, NULL, FILE_CURRENT); + if (pos == INVALID_SET_FILE_POINTER && GetLastError() != NO_ERROR) + return -1; + *off = (int) pos; + return 0; +#endif } int file_lock(Handle fd) diff --git a/src/wal.c b/src/wal.c index 317cc94..53b47d5 100644 --- a/src/wal.c +++ b/src/wal.c @@ -1,6 +1,7 @@ #include #include #include +#include #include "wal.h" #include "file_system.h" @@ -42,66 +43,255 @@ typedef struct { } WALEntry; +static int write_exact(Handle handle, char *src, int len) +{ + int copied = 0; + while (copied < len) { + int ret = file_write(handle, src + copied, len - copied); + if (ret < 0) + return -1; + copied += ret; + } + return 0; +} + typedef struct { + Handle handle; } WriteSnapshotContext; static int serialize_callback(char *src, int num, void *data) { WriteSnapshotContext *wsc = data; - assert(0); // TODO + return write_exact(wsc->handle, src, num); } -static int write_snapshot(FileTree *file_tree) +static int write_snapshot(FileTree *file_tree, Handle handle) { WriteSnapshotContext wsc; + wsc.handle = handle; if (file_tree_serialize(file_tree, serialize_callback, &wsc) < 0) return -1; return 0; } typedef struct { + Handle handle; } ReadSnapshotContext; static int deserialize_callback(char *dst, int num, void *data) { ReadSnapshotContext *rsc = data; - assert(0); // TODO + int copied = 0; + while (copied < num) { + int ret = file_read(rsc->handle, dst + copied, num - copied); + if (ret <= 0) + return -1; + copied += ret; + } + return copied; } static int read_snapshot(FileTree *file_tree, Handle handle) { ReadSnapshotContext rsc; + rsc.handle = handle; int num = file_tree_deserialize(file_tree, deserialize_callback, &rsc); if (num < 0) return -1; - return 0; + return num; } static int swap_file(WAL *wal) { - // TODO: - // - Create a new temporary file - // - Write the WAL file header - // - Serialize the current file tree - // - Rename the temporary file to a name such that it's the next log file that will be used - // - Delete the old log file - // NOTE: - // - The lock will need to be acquired at some point + // Create a temporary file path + char temp_path_buf[1<<10]; + if (wal->file_path.len + 5 > (int) sizeof(temp_path_buf)) + return -1; - assert(0); // TODO + memcpy(temp_path_buf, wal->file_path.ptr, wal->file_path.len); + memcpy(temp_path_buf + wal->file_path.len, ".tmp", 4); + temp_path_buf[wal->file_path.len + 4] = '\0'; + + string temp_path = { temp_path_buf, wal->file_path.len + 4 }; + + // Create and open the temporary file + Handle temp_handle; + if (file_open(temp_path, &temp_handle) < 0) + return -1; + + // Write the WAL header + WALHeader header; + header.magic = WAL_MAGIC; + header.version = WAL_VERSION; + header.reserved = 0; + + if (write_exact(temp_handle, (char*) &header, sizeof(header)) < 0) { + file_close(temp_handle); + remove_file_or_dir(temp_path); + return -1; + } + + // Serialize the current file tree to the new file + if (write_snapshot(wal->file_tree, temp_handle) < 0) { + file_close(temp_handle); + remove_file_or_dir(temp_path); + return -1; + } + + // Sync the temporary file to ensure it's written to disk + if (file_sync(temp_handle) < 0) { + file_close(temp_handle); + remove_file_or_dir(temp_path); + return -1; + } + + // Lock the new file before switching + if (file_lock(temp_handle) < 0) { + file_close(temp_handle); + remove_file_or_dir(temp_path); + return -1; + } + + // Unlock and close the old file + file_unlock(wal->handle); + file_close(wal->handle); + + // Rename the temporary file to replace the old file + if (rename_file_or_dir(temp_path, wal->file_path) < 0) { + file_unlock(temp_handle); + file_close(temp_handle); + return -1; + } + + // Update the WAL to use the new file handle + wal->handle = temp_handle; + wal->entry_count = 0; + + return 0; +} + +static int read_exact(Handle handle, char *dst, int len) +{ + int copied = 0; + while (copied < len) { + int ret = file_read(handle, dst + copied, len - copied); + if (ret < 0) + return -1; + if (ret == 0) + return 0; // EOF + copied += ret; + } + return copied; +} + +static int read_u8(Handle handle, uint8_t *value) +{ + return read_exact(handle, (char*) value, sizeof(*value)); +} + +static int read_u16(Handle handle, uint16_t *value) +{ + return read_exact(handle, (char*) value, sizeof(*value)); +} + +static int read_u32(Handle handle, uint32_t *value) +{ + return read_exact(handle, (char*) value, sizeof(*value)); +} + +static int read_u64(Handle handle, uint64_t *value) +{ + return read_exact(handle, (char*) value, sizeof(*value)); } static int next_entry(Handle handle, WALEntry *entry) { - assert(0); // TODO + static char path_buffer[1<<10]; + static SHA256 prev_hashes_buffer[1<<10]; + static SHA256 next_hashes_buffer[1<<10]; + + uint8_t type; + int ret = read_u8(handle, &type); + if (ret == 0) + return 0; // EOF + if (ret < 0) + return -1; + + entry->type = (WALEntryType) type; + + uint16_t path_len; + if (read_u16(handle, &path_len) <= 0) + return -1; + + if (path_len > sizeof(path_buffer)) + return -1; + + if (read_exact(handle, path_buffer, path_len) <= 0) + return -1; + + entry->path.ptr = path_buffer; + entry->path.len = path_len; + + switch (entry->type) { + case WAL_ENTRY_CREATE: + { + uint8_t is_dir; + if (read_u8(handle, &is_dir) <= 0) + return -1; + entry->is_dir = is_dir; + + if (!is_dir) { + if (read_u64(handle, &entry->chunk_size) <= 0) + return -1; + } else { + entry->chunk_size = 0; + } + } + break; + + case WAL_ENTRY_DELETE: + // No additional fields + break; + + case WAL_ENTRY_WRITE: + { + if (read_u64(handle, &entry->offset) <= 0) + return -1; + if (read_u64(handle, &entry->length) <= 0) + return -1; + if (read_u32(handle, &entry->num_chunks) <= 0) + return -1; + if (read_u32(handle, (uint32_t*) &entry->chunk_size) <= 0) + return -1; + + if (entry->num_chunks > sizeof(prev_hashes_buffer) / sizeof(SHA256)) + return -1; + + if (read_exact(handle, (char*) prev_hashes_buffer, entry->num_chunks * sizeof(SHA256)) <= 0) + return -1; + if (read_exact(handle, (char*) next_hashes_buffer, entry->num_chunks * sizeof(SHA256)) <= 0) + return -1; + + entry->prev_hashes = prev_hashes_buffer; + entry->next_hashes = next_hashes_buffer; + } + break; + + default: + return -1; + } + + return 1; } int wal_open(WAL *wal, FileTree *file_tree, string file_path, int entry_limit) { wal->entry_count = 0; wal->entry_limit = entry_limit; + wal->file_tree = file_tree; + wal->file_path = file_path; Handle handle; if (file_open(file_path, &handle) < 0) @@ -112,7 +302,41 @@ int wal_open(WAL *wal, FileTree *file_tree, string file_path, int entry_limit) return -1; } - // TODO: If the file didn't exist already, initialize it + // Check if the file is empty (newly created) and initialize it + size_t size; + if (file_size(handle, &size) < 0) { + file_close(handle); + return -1; + } + + if (size == 0) { + // Initialize a new WAL file + WALHeader header; + header.magic = WAL_MAGIC; + header.version = WAL_VERSION; + header.reserved = 0; + + if (write_exact(handle, (char*) &header, sizeof(header)) < 0) { + file_close(handle); + return -1; + } + + if (write_snapshot(file_tree, handle) < 0) { + file_close(handle); + return -1; + } + + if (file_sync(handle) < 0) { + file_close(handle); + return -1; + } + + // Reset to beginning after initialization + if (file_set_offset(handle, 0) < 0) { + file_close(handle); + return -1; + } + } // Read file header // NOTE: For now we don't worry about fixing endianess @@ -153,12 +377,12 @@ int wal_open(WAL *wal, FileTree *file_tree, string file_path, int entry_limit) return -1; } - // Not restore the offset to the correct position. + // Now restore the offset to the correct position. if (num > INT_MAX - saved_offset) { file_close(handle); return -1; } - if (file_set_offset(wal->handle, saved_offset + num) < 0) { + if (file_set_offset(handle, saved_offset + num) < 0) { file_close(handle); return -1; } @@ -192,6 +416,8 @@ int wal_open(WAL *wal, FileTree *file_tree, string file_path, int entry_limit) wal->entry_count++; } + wal->handle = handle; + if (wal->entry_count >= wal->entry_limit) { if (swap_file(wal) < 0) return -1; @@ -205,18 +431,6 @@ void wal_close(WAL *wal) file_close(wal->handle); } -static int write_exact(Handle handle, char *src, int len) -{ - int copied = 0; - while (copied < len) { - int ret = file_write(handle, src + copied, len - copied); - if (ret < 0) - return -1; - copied += ret; - } - return 0; -} - static int write_u8(Handle handle, uint8_t value) { return write_exact(handle, (char*) &value, sizeof(value)); @@ -227,6 +441,11 @@ static int write_u16(Handle handle, uint16_t value) return write_exact(handle, (char*) &value, sizeof(value)); } +static int write_u32(Handle handle, uint32_t value) +{ + return write_exact(handle, (char*) &value, sizeof(value)); +} + static int write_u64(Handle handle, uint64_t value) { return write_exact(handle, (char*) &value, sizeof(value)); @@ -243,6 +462,7 @@ static int append_begin(WAL *wal) if (swap_file(wal) < 0) return -1; } + return 0; } static int append_end(WAL *wal) @@ -302,7 +522,24 @@ int wal_append_write(WAL *wal, string path, uint64_t off, if (append_begin(wal) < 0) return -1; - assert(0); // TODO + if (write_u8(wal->handle, WAL_ENTRY_WRITE) < 0) + return -1; + if (write_u16(wal->handle, path.len) < 0) + return -1; + if (write_str(wal->handle, path) < 0) + return -1; + if (write_u64(wal->handle, off) < 0) + return -1; + if (write_u64(wal->handle, len) < 0) + return -1; + if (write_u32(wal->handle, num_chunks) < 0) + return -1; + if (write_u32(wal->handle, chunk_size) < 0) + return -1; + if (write_exact(wal->handle, (char*) prev_hashes, num_chunks * sizeof(SHA256)) < 0) + return -1; + if (write_exact(wal->handle, (char*) hashes, num_chunks * sizeof(SHA256)) < 0) + return -1; if (append_end(wal) < 0) return -1; diff --git a/src/wal.h b/src/wal.h index 38feafa..6d7e9fc 100644 --- a/src/wal.h +++ b/src/wal.h @@ -6,6 +6,8 @@ typedef struct { Handle handle; + FileTree *file_tree; + string file_path; int entry_count; int entry_limit; } WAL; From e8d1e059c35a1c86359984abed28f2af94619912 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Nov 2025 21:12:33 +0000 Subject: [PATCH 2/3] Improve WAL robustness: fix lifetime issues and use dynamic allocation This commit addresses several important robustness issues in the WAL implementation: 1. Fixed file_path lifetime issue: - The file_path argument to wal_open() may not have the same lifetime as the WAL structure - Now allocate and copy the path string to ensure WAL owns its data - Properly free the allocated path in wal_close() - Added error cleanup path in wal_open() to prevent memory leaks 2. Replaced static arrays with dynamic allocation in next_entry(): - Static arrays were not thread-safe and had limited lifetimes - Now dynamically allocate path buffer and hash buffers using sys_malloc - Caller must free allocated fields after using the entry - Added proper error cleanup to free allocations on failure - Updated wal_open() to free entry fields after processing each entry 3. Improved swap_file() for cross-platform compatibility: - On Unix/Linux: rename() atomically replaces the destination file - On Windows: rename() doesn't overwrite, so delete old file first - Added platform-specific handling with #ifdef _WIN32 - Ensures WAL rotation works correctly on both platforms 4. Added system.h include for sys_malloc/sys_free definitions These changes ensure proper memory management, prevent leaks, and make the WAL implementation more robust across different platforms. --- src/wal.c | 137 ++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 101 insertions(+), 36 deletions(-) diff --git a/src/wal.c b/src/wal.c index 53b47d5..8331a70 100644 --- a/src/wal.c +++ b/src/wal.c @@ -6,6 +6,7 @@ #include "wal.h" #include "file_system.h" #include "file_tree.h" +#include "system.h" #define WAL_MAGIC 0xcafebebe #define WAL_VERSION 1 @@ -158,6 +159,18 @@ static int swap_file(WAL *wal) file_unlock(wal->handle); file_close(wal->handle); + // On Unix, rename() atomically replaces the destination file. + // On Windows, rename() doesn't overwrite, so we need to delete first. +#ifdef _WIN32 + // Remove the old file before renaming on Windows + if (remove_file_or_dir(wal->file_path) < 0) { + file_unlock(temp_handle); + file_close(temp_handle); + remove_file_or_dir(temp_path); + return -1; + } +#endif + // Rename the temporary file to replace the old file if (rename_file_or_dir(temp_path, wal->file_path) < 0) { file_unlock(temp_handle); @@ -208,9 +221,10 @@ static int read_u64(Handle handle, uint64_t *value) static int next_entry(Handle handle, WALEntry *entry) { - static char path_buffer[1<<10]; - static SHA256 prev_hashes_buffer[1<<10]; - static SHA256 next_hashes_buffer[1<<10]; + // Initialize pointers to NULL for cleanup on error + entry->path.ptr = NULL; + entry->prev_hashes = NULL; + entry->next_hashes = NULL; uint8_t type; int ret = read_u8(handle, &type); @@ -225,11 +239,15 @@ static int next_entry(Handle handle, WALEntry *entry) if (read_u16(handle, &path_len) <= 0) return -1; - if (path_len > sizeof(path_buffer)) + // Dynamically allocate path buffer + char *path_buffer = sys_malloc(path_len); + if (!path_buffer) return -1; - if (read_exact(handle, path_buffer, path_len) <= 0) + if (read_exact(handle, path_buffer, path_len) <= 0) { + sys_free(path_buffer); return -1; + } entry->path.ptr = path_buffer; entry->path.len = path_len; @@ -239,12 +257,12 @@ static int next_entry(Handle handle, WALEntry *entry) { uint8_t is_dir; if (read_u8(handle, &is_dir) <= 0) - return -1; + goto cleanup_error; entry->is_dir = is_dir; if (!is_dir) { if (read_u64(handle, &entry->chunk_size) <= 0) - return -1; + goto cleanup_error; } else { entry->chunk_size = 0; } @@ -258,21 +276,36 @@ static int next_entry(Handle handle, WALEntry *entry) case WAL_ENTRY_WRITE: { if (read_u64(handle, &entry->offset) <= 0) - return -1; + goto cleanup_error; if (read_u64(handle, &entry->length) <= 0) - return -1; + goto cleanup_error; if (read_u32(handle, &entry->num_chunks) <= 0) - return -1; + goto cleanup_error; if (read_u32(handle, (uint32_t*) &entry->chunk_size) <= 0) - return -1; + goto cleanup_error; - if (entry->num_chunks > sizeof(prev_hashes_buffer) / sizeof(SHA256)) - return -1; + // Dynamically allocate hash buffers + SHA256 *prev_hashes_buffer = sys_malloc(entry->num_chunks * sizeof(SHA256)); + SHA256 *next_hashes_buffer = sys_malloc(entry->num_chunks * sizeof(SHA256)); - if (read_exact(handle, (char*) prev_hashes_buffer, entry->num_chunks * sizeof(SHA256)) <= 0) - return -1; - if (read_exact(handle, (char*) next_hashes_buffer, entry->num_chunks * sizeof(SHA256)) <= 0) - return -1; + if (!prev_hashes_buffer || !next_hashes_buffer) { + if (prev_hashes_buffer) + sys_free(prev_hashes_buffer); + if (next_hashes_buffer) + sys_free(next_hashes_buffer); + goto cleanup_error; + } + + if (read_exact(handle, (char*) prev_hashes_buffer, entry->num_chunks * sizeof(SHA256)) <= 0) { + sys_free(prev_hashes_buffer); + sys_free(next_hashes_buffer); + goto cleanup_error; + } + if (read_exact(handle, (char*) next_hashes_buffer, entry->num_chunks * sizeof(SHA256)) <= 0) { + sys_free(prev_hashes_buffer); + sys_free(next_hashes_buffer); + goto cleanup_error; + } entry->prev_hashes = prev_hashes_buffer; entry->next_hashes = next_hashes_buffer; @@ -280,10 +313,19 @@ static int next_entry(Handle handle, WALEntry *entry) break; default: - return -1; + goto cleanup_error; } return 1; + +cleanup_error: + if (entry->path.ptr) + sys_free((char*) entry->path.ptr); + if (entry->prev_hashes) + sys_free(entry->prev_hashes); + if (entry->next_hashes) + sys_free(entry->next_hashes); + return -1; } int wal_open(WAL *wal, FileTree *file_tree, string file_path, int entry_limit) @@ -291,22 +333,30 @@ int wal_open(WAL *wal, FileTree *file_tree, string file_path, int entry_limit) wal->entry_count = 0; wal->entry_limit = entry_limit; wal->file_tree = file_tree; - wal->file_path = file_path; + wal->file_path.ptr = NULL; + + // Copy file_path since the passed string may not have the same lifetime as WAL + char *path_copy = sys_malloc(file_path.len); + if (!path_copy) + return -1; + memcpy(path_copy, file_path.ptr, file_path.len); + wal->file_path.ptr = path_copy; + wal->file_path.len = file_path.len; Handle handle; if (file_open(file_path, &handle) < 0) - return -1; + goto error_cleanup_path; if (file_lock(handle) < 0) { file_close(handle); - return -1; + goto error_cleanup_path; } // Check if the file is empty (newly created) and initialize it size_t size; if (file_size(handle, &size) < 0) { file_close(handle); - return -1; + goto error_cleanup_path; } if (size == 0) { @@ -318,23 +368,23 @@ int wal_open(WAL *wal, FileTree *file_tree, string file_path, int entry_limit) if (write_exact(handle, (char*) &header, sizeof(header)) < 0) { file_close(handle); - return -1; + goto error_cleanup_path; } if (write_snapshot(file_tree, handle) < 0) { file_close(handle); - return -1; + goto error_cleanup_path; } if (file_sync(handle) < 0) { file_close(handle); - return -1; + goto error_cleanup_path; } // Reset to beginning after initialization if (file_set_offset(handle, 0) < 0) { file_close(handle); - return -1; + goto error_cleanup_path; } } @@ -345,7 +395,7 @@ int wal_open(WAL *wal, FileTree *file_tree, string file_path, int entry_limit) int ret = file_read(handle, (char*) &header + copied, (int) sizeof(header) - copied); if (ret <= 0) { file_close(handle); // TODO: what happens if I close a file without unlocking it? - return -1; + goto error_cleanup_path; } copied += ret; } @@ -353,11 +403,11 @@ int wal_open(WAL *wal, FileTree *file_tree, string file_path, int entry_limit) // Validate header fields if (header.magic != WAL_MAGIC) { file_close(handle); - return -1; + goto error_cleanup_path; } if (header.version != WAL_VERSION) { file_close(handle); - return -1; + goto error_cleanup_path; } // The read_snapshot function may read more @@ -368,23 +418,23 @@ int wal_open(WAL *wal, FileTree *file_tree, string file_path, int entry_limit) int saved_offset; if (file_get_offset(handle, &saved_offset) < 0) { file_close(handle); - return -1; + goto error_cleanup_path; } int num = read_snapshot(file_tree, handle); if (num < 0) { file_close(handle); - return -1; + goto error_cleanup_path; } // Now restore the offset to the correct position. if (num > INT_MAX - saved_offset) { file_close(handle); - return -1; + goto error_cleanup_path; } if (file_set_offset(handle, saved_offset + num) < 0) { file_close(handle); - return -1; + goto error_cleanup_path; } WALEntry entry; @@ -395,7 +445,7 @@ int wal_open(WAL *wal, FileTree *file_tree, string file_path, int entry_limit) break; if (ret < 0) { file_close(handle); - return -1; + goto error_cleanup_path; } assert(ret == 1); @@ -413,22 +463,37 @@ int wal_open(WAL *wal, FileTree *file_tree, string file_path, int entry_limit) UNREACHABLE; } + // Free dynamically allocated fields from next_entry + if (entry.path.ptr) + sys_free((char*) entry.path.ptr); + if (entry.prev_hashes) + sys_free(entry.prev_hashes); + if (entry.next_hashes) + sys_free(entry.next_hashes); + wal->entry_count++; } wal->handle = handle; if (wal->entry_count >= wal->entry_limit) { - if (swap_file(wal) < 0) - return -1; + if (swap_file(wal) < 0) { + goto error_cleanup_path; + } } return 0; + +error_cleanup_path: + sys_free(path_copy); + return -1; } void wal_close(WAL *wal) { file_unlock(wal->handle); file_close(wal->handle); + if (wal->file_path.ptr) + sys_free((char*) wal->file_path.ptr); } static int write_u8(Handle handle, uint8_t value) From e195c3b3b2cb16a63992a76334ee4abe84bda95d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Nov 2025 21:17:11 +0000 Subject: [PATCH 3/3] Fix critical data loss bug in Windows WAL rotation CRITICAL BUG FIX: The previous Windows implementation had a fatal flaw where it deleted the old WAL file before renaming, creating a window where all data could be lost if the rename failed. Previous (BROKEN) Windows code: remove_file_or_dir(wal->file_path); // Delete old file rename(...); // <-- If this fails, we've lost all data! New implementation uses MoveFileExW with MOVEFILE_REPLACE_EXISTING: - Windows: MoveFileExW(..., MOVEFILE_REPLACE_EXISTING) atomically replaces the destination file, matching Unix semantics - Unix/Linux: rename() continues to atomically replace as before - Both platforms now have atomic file replacement with no data loss window This ensures durability on both platforms - if the operation fails at any point, we still have either the old file or the new file, never losing all data. --- src/wal.c | 46 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/src/wal.c b/src/wal.c index 8331a70..dfc98e0 100644 --- a/src/wal.c +++ b/src/wal.c @@ -159,11 +159,42 @@ static int swap_file(WAL *wal) file_unlock(wal->handle); file_close(wal->handle); - // On Unix, rename() atomically replaces the destination file. - // On Windows, rename() doesn't overwrite, so we need to delete first. + // Atomically rename the temporary file to replace the old file + // On Unix: rename() atomically replaces the destination + // On Windows: we use MoveFileEx with MOVEFILE_REPLACE_EXISTING for atomicity #ifdef _WIN32 - // Remove the old file before renaming on Windows - if (remove_file_or_dir(wal->file_path) < 0) { + // On Windows, use MoveFileEx for atomic replace + char old_path_zt[1<<10]; + if (wal->file_path.len >= (int) sizeof(old_path_zt)) { + file_unlock(temp_handle); + file_close(temp_handle); + remove_file_or_dir(temp_path); + return -1; + } + memcpy(old_path_zt, wal->file_path.ptr, wal->file_path.len); + old_path_zt[wal->file_path.len] = '\0'; + + WCHAR old_path_w[MAX_PATH]; + WCHAR temp_path_w[MAX_PATH]; + + if (!MultiByteToWideChar(CP_UTF8, 0, old_path_zt, -1, old_path_w, MAX_PATH) || + !MultiByteToWideChar(CP_UTF8, 0, temp_path_buf, -1, temp_path_w, MAX_PATH)) { + file_unlock(temp_handle); + file_close(temp_handle); + remove_file_or_dir(temp_path); + return -1; + } + + // MOVEFILE_REPLACE_EXISTING allows atomic overwrite + if (!MoveFileExW(temp_path_w, old_path_w, MOVEFILE_REPLACE_EXISTING)) { + file_unlock(temp_handle); + file_close(temp_handle); + remove_file_or_dir(temp_path); + return -1; + } +#else + // On Unix/Linux, rename() atomically replaces the destination + if (rename_file_or_dir(temp_path, wal->file_path) < 0) { file_unlock(temp_handle); file_close(temp_handle); remove_file_or_dir(temp_path); @@ -171,13 +202,6 @@ static int swap_file(WAL *wal) } #endif - // Rename the temporary file to replace the old file - if (rename_file_or_dir(temp_path, wal->file_path) < 0) { - file_unlock(temp_handle); - file_close(temp_handle); - return -1; - } - // Update the WAL to use the new file handle wal->handle = temp_handle; wal->entry_count = 0;