Skip to content

Commit 9ccd975

Browse files
justrachclaude
andcommitted
fix: make mmap capacity atomic and prevent truncated JSON responses
mmap.zig: capacity was a plain usize read by at()/slice() without locks while grow() writes it from another thread — a formal data race. Now uses std.atomic.Value(usize) with acquire/release ordering. The pre-reserved VA range design means the base pointer is stable, but capacity must be visible to readers after grow extends the mapping. server.zig: scan and search handlers used a 256-byte headroom check before writing each doc, but docs can exceed 256 bytes (code files), causing partial JSON objects in the response buffer. Now saves the stream position before each doc and rewinds on overflow, ensuring only complete JSON objects appear in truncated responses. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 547e3de commit 9ccd975

3 files changed

Lines changed: 32 additions & 17 deletions

File tree

src/page.zig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ pub const PageFile = struct {
7878
// Extend the file.
7979
const pno = self.next_alloc.fetchAdd(1, .seq_cst);
8080
const needed = (@as(usize, pno) + 1) * PAGE_SIZE;
81-
if (needed > self.mm.capacity) try self.mm.grow(needed);
81+
if (needed > self.mm.capacity.load(.acquire)) try self.mm.grow(needed);
8282

8383
const ph = self.pageHeader(pno);
8484
ph.* = std.mem.zeroes(PageHeader);

src/server.zig

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -631,8 +631,7 @@ fn handleScan(srv: *Server, tenant_id: []const u8, col_name: []const u8, query_s
631631
.{ esc_tid, esc_col, result.docs.len }) catch {};
632632
var truncated = false;
633633
for (result.docs, 0..) |d, i| {
634-
// Stop writing if buffer is nearly full to avoid truncated JSON.
635-
if (fbs.pos + 256 >= MAX_BODY) { truncated = true; break; }
634+
const saved_pos = fbs.pos;
636635
if (i > 0) w.writeByte(',') catch {};
637636
var esc_dk_buf: [1024]u8 = undefined;
638637
const esc_dk = jsonEscape(d.key, &esc_dk_buf);
@@ -649,6 +648,13 @@ fn handleScan(srv: *Server, tenant_id: []const u8, col_name: []const u8, query_s
649648
}
650649
w.writeAll("\"}") catch {};
651650
}
651+
// If this doc pushed us near the buffer limit, rewind to discard the
652+
// partial write and stop — this prevents broken JSON in the response.
653+
if (fbs.pos + 64 >= MAX_BODY) {
654+
fbs.pos = saved_pos;
655+
truncated = true;
656+
break;
657+
}
652658
}
653659
w.writeAll("]}") catch {};
654660
if (truncated) {
@@ -692,7 +698,7 @@ fn handleSearch(srv: *Server, tenant_id: []const u8, col_name: []const u8, query
692698
.{ result.docs.len, result.candidate_paths.len, col.docCount(), result.total_files }) catch {};
693699
var truncated = false;
694700
for (result.docs, 0..) |d, i| {
695-
if (fbs.pos + 256 >= MAX_BODY) { truncated = true; break; }
701+
const saved_pos = fbs.pos;
696702
if (i > 0) w.writeByte(',') catch {};
697703
// Output value as valid JSON — objects/arrays as-is, strings quoted
698704
var esc_dk_buf: [1024]u8 = undefined;
@@ -711,6 +717,11 @@ fn handleSearch(srv: *Server, tenant_id: []const u8, col_name: []const u8, query
711717
}
712718
w.writeAll("\"}") catch {};
713719
}
720+
if (fbs.pos + 64 >= MAX_BODY) {
721+
fbs.pos = saved_pos;
722+
truncated = true;
723+
break;
724+
}
714725
}
715726
w.writeAll("]}") catch {};
716727
if (truncated) {

src/storage/mmap.zig

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@ pub const MmapFile = struct {
3434
/// concurrent readers (no lock needed thanks to the stable VA range).
3535
len: std.atomic.Value(usize),
3636
/// How many bytes of the VA range are currently file-backed and mapped
37-
/// PROT_READ|PROT_WRITE. Protected by `grow_mu` (only grow mutates it).
38-
capacity: usize,
37+
/// PROT_READ|PROT_WRITE. Atomic so concurrent at()/slice() callers
38+
/// can read without holding grow_mu.
39+
capacity: std.atomic.Value(usize),
3940
/// Serialises concurrent grow() calls. NOT needed for readers.
4041
grow_mu: std.Thread.Mutex,
4142

@@ -80,7 +81,7 @@ pub const MmapFile = struct {
8081
.fd = fd,
8182
.ptr = base_ptr,
8283
.len = std.atomic.Value(usize).init(existing),
83-
.capacity = capacity,
84+
.capacity = std.atomic.Value(usize).init(capacity),
8485
.grow_mu = .{},
8586
};
8687
}
@@ -93,12 +94,14 @@ pub const MmapFile = struct {
9394

9495
// ── Sync / Checkpoint ─────────────────────────────────────────────────────
9596

96-
pub fn syncAsync(self: MmapFile) void {
97-
posix.msync(@alignCast(self.ptr[0..self.capacity]), posix.MSF.ASYNC) catch {};
97+
pub fn syncAsync(self: *MmapFile) void {
98+
const cap = self.capacity.load(.acquire);
99+
posix.msync(@alignCast(self.ptr[0..cap]), posix.MSF.ASYNC) catch {};
98100
}
99101

100-
pub fn syncSync(self: MmapFile) !void {
101-
try posix.msync(@alignCast(self.ptr[0..self.capacity]), posix.MSF.SYNC);
102+
pub fn syncSync(self: *MmapFile) !void {
103+
const cap = self.capacity.load(.acquire);
104+
try posix.msync(@alignCast(self.ptr[0..cap]), posix.MSF.SYNC);
102105
}
103106

104107
// ── Grow ──────────────────────────────────────────────────────────────────
@@ -107,7 +110,7 @@ pub const MmapFile = struct {
107110
/// extends the file and maps the new pages into the pre-reserved VA
108111
/// range with MAP_FIXED. The base pointer never changes.
109112
pub fn grow(self: *MmapFile, needed_len: usize) !void {
110-
if (needed_len <= self.capacity) {
113+
if (needed_len <= self.capacity.load(.acquire)) {
111114
// Capacity is sufficient — just advance the logical length.
112115
self.len.store(needed_len, .release);
113116
return;
@@ -118,12 +121,13 @@ pub const MmapFile = struct {
118121
defer self.grow_mu.unlock();
119122

120123
// Re-check after acquiring lock (another thread may have grown).
121-
if (needed_len <= self.capacity) {
124+
const cur_cap = self.capacity.load(.acquire);
125+
if (needed_len <= cur_cap) {
122126
self.len.store(needed_len, .release);
123127
return;
124128
}
125129

126-
const old_cap = self.capacity;
130+
const old_cap = cur_cap;
127131
const new_cap = alignUp(needed_len + GROW_CHUNK, OS_PAGE_ALIGN);
128132

129133
std.debug.assert(new_cap <= MAX_VA_SIZE);
@@ -141,7 +145,7 @@ pub const MmapFile = struct {
141145
self.fd, @intCast(old_cap),
142146
);
143147

144-
self.capacity = new_cap;
148+
self.capacity.store(new_cap, .release);
145149
self.len.store(needed_len, .release);
146150
}
147151

@@ -150,14 +154,14 @@ pub const MmapFile = struct {
150154
/// Return a pointer to the record at byte offset `off`.
151155
/// Lock-free: the base pointer is stable for the lifetime of the mapping.
152156
pub fn at(self: *MmapFile, comptime T: type, off: usize) *T {
153-
std.debug.assert(off + @sizeOf(T) <= self.capacity);
157+
std.debug.assert(off + @sizeOf(T) <= self.capacity.load(.acquire));
154158
return @alignCast(@ptrCast(&self.ptr[off]));
155159
}
156160

157161
/// Return a slice of T starting at byte offset `off`, `count` elements.
158162
/// Lock-free: the base pointer is stable for the lifetime of the mapping.
159163
pub fn slice(self: *MmapFile, comptime T: type, off: usize, count: usize) []T {
160-
std.debug.assert(off + count * @sizeOf(T) <= self.capacity);
164+
std.debug.assert(off + count * @sizeOf(T) <= self.capacity.load(.acquire));
161165
return @as([*]T, @alignCast(@ptrCast(&self.ptr[off])))[0..count];
162166
}
163167

0 commit comments

Comments
 (0)