Artifact 77fca142f250c70e757081d5c2c9751e3d1bdd950092d7bc4d608b28f964a33f:
- File
src/store_write_guard_test.zig
— part of check-in
[5c445bb3f3]
at
2026-09-28 02:39:03
on branch trunk
— Take the writer lock inside LogStore.open and test the lock class
LogStore.open now takes the exclusive lock before it reads the file's length and scans for the end, and a read-only handle refuses to append. purge undo opens the store through the shared writer path.
A new DST sweep runs two to four writers interleaved against one log through a file that models the lock. It flags unlocked or misplaced appends as they happen, checks each writer sees everything committed before it locked and reads back its own writes, including deltas. A source check fails on any write-mode open of the store outside openStoreAppend. An e2e test races commits, tags and user-table writes on one store and counts what landed. (user: dmestas, size: 6188) [annotate] [blame] [check-ins using]
//! Every writer reaches the repository file through `cli.openStoreAppend`. //! //! The file is one append-only log, and two writers that are not serialised //! both append at the end they scanned: the second writes over the first with //! frames that are complete and valid, which nothing afterwards can detect. //! `LogStore.open` takes the writer lock itself and a reader's handle cannot //! append, so a writer that goes through `LogStore` is safe however it opened //! the file. What remains is code that opens the store for writing and then //! writes to it some other way. This walks the source and fails on any //! write-mode open of the store outside `openStoreAppend`, so that shape can //! only arrive with a deliberate change here. //! //! Test code is exempt: tests open stores directly to build fixtures and to //! stand in for a competing writer. So are the benchmark tools, which are not //! part of the `zit` binary. const std = @import("std"); const repo_options = @import("repo_options"); /// Names the source uses for the repository file. const store_names = [_][]const u8{ "repo_filename", "repo_marker", "\".zit\"" }; const write_modes = [_][]const u8{ ".mode = .read_write", ".mode = .write_only" }; /// The places allowed to open the store for writing. The first is the rule; /// the rest are test helpers that live outside `test` blocks, called only by /// tests to stand in for a writer. const Allowed = struct { file: []const u8, function: []const u8 }; const allowed = [_]Allowed{ .{ .file = "cli.zig", .function = "openStoreAppend" }, .{ .file = "cmd/sync.zig", .function = "serveAppendFrame" }, .{ .file = "cmd/sync.zig", .function = "serveSeedRepo" }, .{ .file = "cmd/sync.zig", .function = "d4OpenLog" }, }; fn isAllowed(path: []const u8, function: []const u8) bool { for (allowed) |a| if (std.mem.eql(u8, path, a.file) and std.mem.eql(u8, function, a.function)) return true; return false; } const excluded_prefixes = [_][]const u8{"sim/"}; const excluded_files = [_][]const u8{ "e2e.zig", "smoke.zig", "dst.zig" }; /// The walk must see at least this many files, or it did not see the tree. const min_expected_files = 30; const max_source_bytes = 8 * 1024 * 1024; fn isExcluded(path: []const u8) bool { if (std.mem.endsWith(u8, path, "_test.zig")) return true; if (std.mem.endsWith(u8, path, "_benchmark.zig")) return true; for (excluded_prefixes) |p| if (std.mem.startsWith(u8, path, p)) return true; for (excluded_files) |f| if (std.mem.eql(u8, path, f)) return true; return false; } fn containsAny(line: []const u8, needles: []const []const u8) bool { for (needles) |n| if (std.mem.indexOf(u8, line, n) != null) return true; return false; } /// The function a line sits in, from the nearest preceding `fn NAME(` at file /// or container scope. Good enough for this purpose: the check only needs to /// recognise the one allowed function by name. fn fnName(line: []const u8) ?[]const u8 { const t = std.mem.trimStart(u8, line, " "); const indent = line.len - t.len; if (indent > 4) return null; const rest = if (std.mem.startsWith(u8, t, "pub fn ")) t["pub fn ".len..] else if (std.mem.startsWith(u8, t, "fn ")) t["fn ".len..] else return null; const paren = std.mem.indexOfScalar(u8, rest, '(') orelse return null; return rest[0..paren]; } test "only openStoreAppend opens the repository file for writing" { const io = std.testing.io; var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena_state.deinit(); const arena = arena_state.allocator(); const src_abs = try std.fmt.allocPrint(arena, "{s}/src", .{repo_options.root}); var src_dir = std.Io.Dir.cwd().openDir(io, src_abs, .{ .iterate = true }) catch |err| switch (err) { error.FileNotFound, error.NotDir => return error.SkipZigTest, else => return err, }; defer src_dir.close(io); var scanned: usize = 0; var allowed_seen = false; var violations: usize = 0; var walker = try src_dir.walk(arena); defer walker.deinit(); while (try walker.next(io)) |entry| { if (entry.kind != .file or !std.mem.endsWith(u8, entry.path, ".zig")) continue; if (isExcluded(entry.path)) continue; scanned += 1; const source = try src_dir.readFileAlloc(io, entry.path, arena, .limited(max_source_bytes)); var in_test = false; var current_fn: []const u8 = ""; var line_no: usize = 0; var it = std.mem.splitScalar(u8, source, '\n'); while (it.next()) |line| { line_no += 1; if (!in_test and std.mem.startsWith(u8, line, "test \"")) in_test = true; if (in_test) { if (std.mem.eql(u8, line, "}")) in_test = false; continue; } if (fnName(line)) |name| current_fn = name; if (!containsAny(line, &store_names) or !containsAny(line, &write_modes)) continue; if (isAllowed(entry.path, current_fn)) { if (std.mem.eql(u8, entry.path, allowed[0].file) and std.mem.eql(u8, current_fn, allowed[0].function)) allowed_seen = true; continue; } violations += 1; std.debug.print( "\nstore write guard: src/{s}:{d} ({s}) opens the repository file for writing outside cli.openStoreAppend:\n {s}\n", .{ entry.path, line_no, current_fn, std.mem.trim(u8, line, " ") }, ); } } if (scanned < min_expected_files) { std.debug.print("\nstore write guard: scanned only {d} files under {s}; the walk is not seeing the tree\n", .{ scanned, src_abs }); return error.StoreWriteGuardWalkFailed; } // The allowed site must still match the pattern, or a rename would leave // this check passing while it recognises nothing. if (!allowed_seen) { std.debug.print("\nstore write guard: did not find the write-mode open in {s}'s {s}; update this guard\n", .{ allowed[0].file, allowed[0].function }); return error.StoreWriteGuardVacuous; } try std.testing.expectEqual(@as(usize, 0), violations); }