a73x

29d88ab2

refactor: sockpath follow-through — the seam gets its pin, the optional goes, the vestigial ordering is named

a73x   2026-08-12 19:25

Commit message
refactor: sockpath follow-through — the seam gets its pin, the optional goes, the vestigial ordering is named

PathId had no test of its own: the three server tests that reach it all
go through a full daemon, and none of them ever took the catch arm. One
test now drives the type directly — live socket, deleted path,
successor at the same name — and it runs before server_mod, so the
inversion that used to be reported by a wedge-prone daemon test is now
named by a sub-second one.

path_id sheds its optional. init cannot return without one, so the
else-false arm was unreachable, and what it encoded was exactly the
silent no-unlink of 6090604.

The stat-before-close in deinit is vestigial, and now says so: 8a82225
needed it to fstat the live descriptor, 6090604 removed that stat.
Statting after the close would narrow the race a successor has to claim
the path mid-teardown; banked for M16 rather than taken here.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
build.zig
Old New
@@ -191,8 +191,8 @@ pub fn build(b: *std.Build) void {
191 .root_source_file = b.path("src/sockpath.zig"), 191 .root_source_file = b.path("src/sockpath.zig"),
192 .target = target, 192 .target = target,
193 .optimize = optimize, 193 .optimize = optimize,
194 .link_libc = true,
195 }); 194 });
195 sockpath_mod.addImport("testtmp", testtmp_mod);
196 196
197 const server_mod = b.createModule(.{ 197 const server_mod = b.createModule(.{
198 .root_source_file = b.path("src/server.zig"), 198 .root_source_file = b.path("src/server.zig"),
src/delta.zig
Old New
@@ -1,7 +1,8 @@
1 //! Row-level change tracking behind the delta stream: one content hash per 1 //! The daemon's half of the delta stream. This side holds the authoritative
2 //! viewport row, so an engine update can be sent as just the rows that 2 //! grid and decides WHICH rows a client is missing; the far side is a
3 //! actually changed, and a reattach can be answered with a delta instead of 3 //! replica that applies whatever payload arrives without knowing how it was
4 //! a full snapshot. 4 //! chosen. protocol.zig owns the bytes those payloads are made of — this
5 //! owns which rows go into them.
5 //! 6 //!
6 //! Engine and protocol are the whole of its world — no daemon, no clients, 7 //! Engine and protocol are the whole of its world — no daemon, no clients,
7 //! no sockets — which is what lets the tracker be driven directly by a test 8 //! no sockets — which is what lets the tracker be driven directly by a test
@@ -134,6 +135,8 @@ pub const DeltaTracker = struct {
134 /// the tracker does not know which daemon instance it belongs to. 135 /// the tracker does not know which daemon instance it belongs to.
135 /// 136 ///
136 /// 0 is never serviceable: it is what a client says when it holds 137 /// 0 is never serviceable: it is what a client says when it holds
138 /// nothing. Neither is any seq against an unbuilt tracker — one that has
139 /// never been built, or whose rebuild failed partway, can describe
137 /// nothing. 140 /// nothing.
138 pub fn canServe(self: *const DeltaTracker, have_seq: u64) bool { 141 pub fn canServe(self: *const DeltaTracker, have_seq: u64) bool {
139 return have_seq != 0 and 142 return have_seq != 0 and
src/server.zig
Old New
@@ -218,8 +218,10 @@ pub const Server = struct {
218 listener: std.net.Server, 218 listener: std.net.Server,
219 sock_path: []const u8, 219 sock_path: []const u8,
220 /// What the socket file was when we bound it, so teardown can tell our 220 /// What the socket file was when we bound it, so teardown can tell our
221 /// socket from one that replaced it. See sockpath.PathId. 221 /// socket from one that replaced it. See sockpath.PathId. Not optional:
222 path_id: ?sockpath.PathId, 222 /// init cannot return without one, and an absent-means-false arm is the
223 /// silent no-unlink 6090604 fixed.
224 path_id: sockpath.PathId,
223 /// The attached interactive clients. All of them see every update. 225 /// The attached interactive clients. All of them see every update.
224 clients: [max_clients]?ClientSlot = @splat(null), 226 clients: [max_clients]?ClientSlot = @splat(null),
225 /// How much unsent output one client may accumulate before the daemon 227 /// How much unsent output one client may accumulate before the daemon
@@ -331,7 +333,10 @@ pub const Server = struct {
331 // newer daemon may have replaced the file since we bound it, and 333 // newer daemon may have replaced the file since we bound it, and
332 // deleting that one would hand its clients the same field incident 334 // deleting that one would hand its clients the same field incident
333 // the socket-steal fix exists to prevent. 335 // the socket-steal fix exists to prevent.
334 const ours: bool = if (self.path_id) |id| id.stillAt(self.sock_path) else false; 336 // Statting before the close is vestigial: 8a82225 fstat'd the live
337 // descriptor, 6090604 removed that stat. After the close would narrow
338 // the successor-claims-the-path window — an M16 change, not an M15 one.
339 const ours: bool = self.path_id.stillAt(self.sock_path);
335 self.listener.deinit(); 340 self.listener.deinit();
336 if (ours) std.fs.cwd().deleteFile(self.sock_path) catch {}; 341 if (ours) std.fs.cwd().deleteFile(self.sock_path) catch {};
337 self.tracker.deinit(self.alloc); 342 self.tracker.deinit(self.alloc);
src/sockpath.zig
Old New
@@ -8,6 +8,7 @@
8 //! of the daemon. Nothing here knows a Server exists; a path is all it 8 //! of the daemon. Nothing here knows a Server exists; a path is all it
9 //! takes. 9 //! takes.
10 const std = @import("std"); 10 const std = @import("std");
11 const TmpDir = @import("testtmp").TmpDir;
11 12
12 /// A socket file's identity at the moment it was bound, so teardown can 13 /// A socket file's identity at the moment it was bound, so teardown can
13 /// tell our socket from one that replaced it. 14 /// tell our socket from one that replaced it.
@@ -84,3 +85,38 @@ pub fn claim(path: []const u8) !void {
84 else => |e| return e, 85 else => |e| return e,
85 }; 86 };
86 } 87 }
88
89 test "PathId: names the file it was taken from, not the path, and not a successor" {
90 var tmp = try TmpDir.make();
91 defer tmp.cleanup();
92
93 var buf: [64]u8 = undefined;
94 const sock_path = try std.fmt.bufPrint(&buf, "{s}/id.sock", .{tmp.path()});
95
96 const addr = try std.net.Address.initUnix(sock_path);
97 var listener = try addr.listen(.{});
98 const id = try PathId.of(sock_path);
99
100 // A live bound socket answers for itself — which is the whole reason
101 // this type takes the PATH's dev+ino. The guard it replaced stat'd the
102 // LISTENING DESCRIPTOR, which lives in sockfs, against the path's
103 // ordinary filesystem inode: never equal, so this case was false too,
104 // and the daemon silently stopped unlinking its own socket on every
105 // clean exit (6090604).
106 try std.testing.expect(id.stillAt(sock_path));
107
108 // Nothing at the path: the stat fails, and the answer callers need is
109 // "not ours" rather than an error to handle. The catch arm, which no
110 // test reached before this one.
111 listener.deinit();
112 try std.fs.cwd().deleteFile(sock_path);
113 try std.testing.expect(!id.stillAt(sock_path));
114
115 // A successor binds the same NAME. Same path, different file, and
116 // deleting it would strand the daemon that owns it — the field incident
117 // the whole module exists for, in one line.
118 const addr2 = try std.net.Address.initUnix(sock_path);
119 var successor = try addr2.listen(.{});
120 defer successor.deinit();
121 try std.testing.expect(!id.stillAt(sock_path));
122 }