a73x

b4e2e372

refactor: one borrowed-name birth, one list snapshot, one unwind

a73x   2026-08-29 10:01

Commit message
refactor: one borrowed-name birth, one list snapshot, one unwind

birthTile already dupes a borrowed session name and rebuilds the label,
and frees both when the wall refuses the tile — yet addSessionTile and
pickBirth did all of it by hand, each with its own free on the orelse.
Both now pass `.borrowed = true`, so the ownership rule lives in one
place. Inside, the five hand-written unwinds (tree.remove, and twice the
copies with it) are errdefers on birthTileOrRefuse, which birthTile turns
back into the null its callers act on. Host.snapshotList owns the
list_mu copy the picker and the poll diff each spelled.

Pinned by wall_test_wall (chord birth, nextFreeName, focus after exit),
wall_test_picker (Enter births, auto-open once) and wall_test_host, all
under the leak check; e2e 07_wallcli 5, 12_panes 10, 09_hosts 10.
src/tui/wall_host.zig
Old New
@@ -351,6 +351,17 @@ pub const Host = struct {
351 /// same reason `freeSlot` does: a forgotten slot rewritten while its 351 /// same reason `freeSlot` does: a forgotten slot rewritten while its
352 /// poller still holds the pointer is a thread dialling a replaced spec. 352 /// poller still holds the pointer is a thread dialling a replaced spec.
353 poller_done: std.atomic.Value(bool) = std.atomic.Value(bool).init(false), 353 poller_done: std.atomic.Value(bool) = std.atomic.Value(bool).init(false),
354
355 /// The poller's last answer, copied out from under `list_mu` into the
356 /// caller's buffer: `list` is the poller's own and is overwritten by
357 /// the next poll, so a reader that kept a slice of it would read a
358 /// half-written list.
359 pub fn snapshotList(self: *Host, buf: *[proto.sessions_text_max]u8) []const u8 {
360 self.list_mu.lock();
361 defer self.list_mu.unlock();
362 @memcpy(buf[0..self.list_len], self.list[0..self.list_len]);
363 return buf[0..self.list_len];
364 }
354 }; 365 };
355 366
356 /// Polling, not a push: a subscription is a new daemon concept, and one 367 /// Polling, not a push: a subscription is a new daemon concept, and one
@@ -444,12 +455,7 @@ pub fn applyHostList(
444 const reachable = h.reachable.load(.acquire); 455 const reachable = h.reachable.load(.acquire);
445 var list_buf: [proto.sessions_text_max]u8 = undefined; 456 var list_buf: [proto.sessions_text_max]u8 = undefined;
446 var list: []const u8 = ""; 457 var list: []const u8 = "";
447 if (reachable) { 458 if (reachable) list = h.snapshotList(&list_buf);
448 h.list_mu.lock();
449 @memcpy(list_buf[0..h.list_len], h.list[0..h.list_len]);
450 list = list_buf[0..h.list_len];
451 h.list_mu.unlock();
452 }
453 // Whether the focus was on a real tile when this list arrived. An empty 459 // Whether the focus was on a real tile when this list arrived. An empty
454 // wall has none, and a vanish can take the one there was — either way 460 // wall has none, and a vanish can take the one there was — either way
455 // the wall owes the tile it ends up with a `setFocus`, which is the only 461 // the wall owes the tile it ends up with a `setFocus`, which is the only
src/tui/wall_picker.zig
Old New
@@ -207,19 +207,11 @@ pub fn pickBirth(
207 target.hand.quiet = true; 207 target.hand.quiet = true;
208 } 208 }
209 var list_buf: [proto.sessions_text_max]u8 = undefined; 209 var list_buf: [proto.sessions_text_max]u8 = undefined;
210 h.list_mu.lock(); 210 const list = h.snapshotList(&list_buf);
211 @memcpy(list_buf[0..h.list_len], h.list[0..h.list_len]);
212 const list_len = h.list_len;
213 h.list_mu.unlock();
214 // The daemon's own naming, off the daemon's own list: the name the 211 // The daemon's own naming, off the daemon's own list: the name the
215 // `c` chord would have landed on, reached without a pump to ask. 212 // `c` chord would have landed on, reached without a pump to ask.
216 var name_buf: [proto.session_name_max]u8 = undefined; 213 var name_buf: [proto.session_name_max]u8 = undefined;
217 const name = client.nextFreeName(&name_buf, list_buf[0..list_len]); 214 const name = client.nextFreeName(&name_buf, list);
218 const session = alloc.dupe(u8, name) catch return null;
219 const label = wv.tileLabel(alloc, target, session) catch {
220 alloc.free(session);
221 return null;
222 };
223 const anchor = wall_layout.anchorTile(present[0..live.*], shared.sel); 215 const anchor = wall_layout.anchorTile(present[0..live.*], shared.sel);
224 const has_anchor = wv.presentCount(present[0..live.*]) > 0; 216 const has_anchor = wv.presentCount(present[0..live.*]) > 0;
225 // `-A` is inherited only within one host. A chord inherits it because 217 // `-A` is inherited only within one host. A chord inherits it because
@@ -229,7 +221,9 @@ pub fn pickBirth(
229 const agent = has_anchor and tiles[anchor].host != null and 221 const agent = has_anchor and tiles[anchor].host != null and
230 tiles[anchor].host.? == sel and tiles[anchor].r.agent; 222 tiles[anchor].host.? == sel and tiles[anchor].r.agent;
231 const at = wv.birthTile(alloc, tiles, present, live, shared, .{ 223 const at = wv.birthTile(alloc, tiles, present, live, shared, .{
232 .r = .{ .target = target, .label = label, .session = session, .agent = agent }, 224 // `name` is this stack's buffer until the wall takes the tile —
225 // see `Birth.borrowed`.
226 .r = .{ .target = target, .label = "", .session = name, .agent = agent },
233 .from = anchor, 227 .from = anchor,
234 .place = .beside_focus, 228 .place = .beside_focus,
235 .creates = true, 229 .creates = true,
@@ -238,9 +232,8 @@ pub fn pickBirth(
238 .born_from = if (has_anchor) anchor else null, 232 .born_from = if (has_anchor) anchor else null,
239 .keeps_wall = true, 233 .keeps_wall = true,
240 .host = sel, 234 .host = sel,
235 .borrowed = true,
241 }) orelse { 236 }) orelse {
242 alloc.free(session);
243 alloc.free(label);
244 wv.setNotice(shared, "[no room on the wall for another session]"); 237 wv.setNotice(shared, "[no room on the wall for another session]");
245 return null; 238 return null;
246 }; 239 };
src/tui/wallview.zig
Old New
@@ -983,11 +983,22 @@ pub fn birthTile(
983 shared: *Shared, 983 shared: *Shared,
984 b: Birth, 984 b: Birth,
985 ) ?usize { 985 ) ?usize {
986 return birthTileOrRefuse(alloc, tiles, present, live, shared, b) catch null;
987 }
988
989 fn birthTileOrRefuse(
990 alloc: std.mem.Allocator,
991 tiles: []Tile,
992 present: []bool,
993 live: *usize,
994 shared: *Shared,
995 b: Birth,
996 ) !usize {
986 // The lowest digit a departed tile left behind, before a new one: 997 // The lowest digit a departed tile left behind, before a new one:
987 // create 1 2 3, end 2, create — and the wall says 2, not 4. Reuse, not 998 // create 1 2 3, end 2, create — and the wall says 2, not 4. Reuse, not
988 // renumbering: the tiles that stayed keep the digit their user learned. 999 // renumbering: the tiles that stayed keep the digit their user learned.
989 const reuse = freeSlot(tiles[0..live.*], present[0..live.*]); 1000 const reuse = freeSlot(tiles[0..live.*], present[0..live.*]);
990 if (reuse == null and live.* >= max_tiles) return null; 1001 if (reuse == null and live.* >= max_tiles) return error.WallFull;
991 const new_live = presentCount(present[0..live.*]) + 1; 1002 const new_live = presentCount(present[0..live.*]) + 1;
992 // "Does it fit" has ONE owner, and it is the tree: insert, flatten, 1003 // "Does it fit" has ONE owner, and it is the tree: insert, flatten,
993 // and undo the insert when flatten refuses. Row arithmetic here 1004 // and undo the insert when flatten refuses. Row arithmetic here
@@ -998,27 +1009,24 @@ pub fn birthTile(
998 .beside_focus => if (shared.tree.root == null) 1009 .beside_focus => if (shared.tree.root == null)
999 // A wall whose tiles all arrive from a host's list starts with 1010 // A wall whose tiles all arrive from a host's list starts with
1000 // no tree at all; `insert` has no leaf to sit beside. 1011 // no tree at all; `insert` has no leaf to sit beside.
1001 shared.tree.addFirst(@intCast(at)) catch return null 1012 try shared.tree.addFirst(@intCast(at))
1002 else 1013 else
1003 shared.tree.insert(@intCast(b.from), @intCast(at)) catch return null, 1014 try shared.tree.insert(@intCast(b.from), @intCast(at)),
1004 .right_of => shared.tree.splitRight(@intCast(b.from), @intCast(at)) catch return null, 1015 .right_of => try shared.tree.splitRight(@intCast(b.from), @intCast(at)),
1005 .below => shared.tree.splitBelow(@intCast(b.from), @intCast(at)) catch return null, 1016 .below => try shared.tree.splitBelow(@intCast(b.from), @intCast(at)),
1006 } 1017 }
1007 const flat = shared.tree.flatten( 1018 // Past the insert, so every refusal below puts the tree back exactly
1019 // as the caller found it.
1020 errdefer shared.tree.remove(@intCast(at));
1021 const flat = try shared.tree.flatten(
1008 alloc, 1022 alloc,
1009 shared.size.rows, 1023 shared.size.rows,
1010 shared.size.cols, 1024 shared.size.cols,
1011 wall_layout.wallFloors(new_live), 1025 wall_layout.wallFloors(new_live),
1012 null, 1026 null,
1013 ) catch { 1027 );
1014 shared.tree.remove(@intCast(at));
1015 return null;
1016 };
1017 defer flat.deinit(alloc); 1028 defer flat.deinit(alloc);
1018 const new_rect = flat.rectOf(@intCast(at)) orelse { 1029 const new_rect = flat.rectOf(@intCast(at)) orelse return error.NoRect;
1019 shared.tree.remove(@intCast(at));
1020 return null;
1021 };
1022 // The real rect, not a placeholder: a creating tile puts its rect on 1030 // The real rect, not a placeholder: a creating tile puts its rect on
1023 // the first attach frame and the daemon refuses creates under 1031 // the first attach frame and the daemon refuses creates under
1024 // min_session_rows; a joining tile doorbells a resize on its first 1032 // min_session_rows; a joining tile doorbells a resize on its first
@@ -1032,17 +1040,16 @@ pub fn birthTile(
1032 // against a slot that is exactly as the caller found it. 1040 // against a slot that is exactly as the caller found it.
1033 var r = b.r; 1041 var r = b.r;
1034 if (b.borrowed) { 1042 if (b.borrowed) {
1035 const session = alloc.dupe(u8, b.r.session) catch { 1043 const session = try alloc.dupe(u8, b.r.session);
1036 shared.tree.remove(@intCast(at)); 1044 errdefer alloc.free(session);
1037 return null;
1038 };
1039 r.session = session; 1045 r.session = session;
1040 r.label = tileLabel(alloc, b.r.target, session) catch { 1046 r.label = try tileLabel(alloc, b.r.target, session);
1041 alloc.free(session);
1042 shared.tree.remove(@intCast(at));
1043 return null;
1044 };
1045 } 1047 }
1048 // Only the copies THIS call made: a caller that owns them frees its own.
1049 errdefer if (b.borrowed) {
1050 alloc.free(r.session);
1051 alloc.free(r.label);
1052 };
1046 // The departed tile's copies go with its digit. Every tile owns these 1053 // The departed tile's copies go with its digit. Every tile owns these
1047 // two — `run` dupes even the entry tile's session for this — so a 1054 // two — `run` dupes even the entry tile's session for this — so a
1048 // reused slot that kept them would leak one label and one name per 1055 // reused slot that kept them would leak one label and one name per
@@ -1051,17 +1058,9 @@ pub fn birthTile(
1051 alloc.free(tiles[at].r.session); 1058 alloc.free(tiles[at].r.session);
1052 alloc.free(tiles[at].r.label); 1059 alloc.free(tiles[at].r.label);
1053 } 1060 }
1054 initTile(&tiles[at], r, new_rect, shared, at, if (reuse == null) .fresh else .kept) catch { 1061 // Only a `.fresh` doorbell can fail, and it fails before the slot is
1055 // Only a `.fresh` doorbell can fail, and it fails before the slot 1062 // written, so the errdefers above are the whole unwind.
1056 // is written: there is nothing here to undo but the tree and the 1063 try initTile(&tiles[at], r, new_rect, shared, at, if (reuse == null) .fresh else .kept);
1057 // copies just made.
1058 shared.tree.remove(@intCast(at));
1059 if (b.borrowed) {
1060 alloc.free(r.session);
1061 alloc.free(r.label);
1062 }
1063 return null;
1064 };
1065 // The caller's row of the birth table, and the whole of what separates 1064 // The caller's row of the birth table, and the whole of what separates
1066 // the roads: a chord row inherits its target and agent and CREATES, a 1065 // the roads: a chord row inherits its target and agent and CREATES, a
1067 // poll row joins a session the daemon already has. The pump spawn stays 1066 // poll row joins a session the daemon already has. The pump spawn stays
@@ -1102,33 +1101,22 @@ fn addSessionTile(
1102 if (!sameTarget(t.r.target, target)) continue; 1101 if (!sameTarget(t.r.target, target)) continue;
1103 if (std.mem.eql(u8, proto.resolveName(t.r.session), want)) return .{ .moved = i }; 1102 if (std.mem.eql(u8, proto.resolveName(t.r.session), want)) return .{ .moved = i };
1104 } 1103 }
1105 // The tile's own copies. `run`'s allocator outlives the process (it
1106 // never returns on the success path), which is what lets a pump hold
1107 // these slices for as long as it lives.
1108 const session = alloc.dupe(u8, want) catch return .full;
1109 const label = tileLabel(alloc, target, want) catch {
1110 alloc.free(session);
1111 return .full;
1112 };
1113 const at = birthTile(alloc, tiles, present, live, shared, .{ 1104 const at = birthTile(alloc, tiles, present, live, shared, .{
1114 // The offer is inherited from the tile this one grew out of. Same 1105 // The offer is inherited from the tile this one grew out of. Same
1115 // target, so `-A` exposes nothing the user has not already exposed 1106 // target, so `-A` exposes nothing the user has not already exposed
1116 // to that host — and a chord-made tile has no command line to spell 1107 // to that host — and a chord-made tile has no command line to spell
1117 // the flag on, so not inheriting would silently end the forwarding 1108 // the flag on, so not inheriting would silently end the forwarding
1118 // at the first session switch. 1109 // at the first session switch.
1119 .r = .{ .target = target, .label = label, .session = session, .agent = tiles[from].r.agent }, 1110 // `want` borrows the caller's name: the wall makes its own copies
1111 // only if it keeps the tile — see `Birth.borrowed`.
1112 .r = .{ .target = target, .label = "", .session = want, .agent = tiles[from].r.agent },
1120 .from = from, 1113 .from = from,
1121 .place = place, 1114 .place = place,
1122 .creates = true, 1115 .creates = true,
1123 .born_from = from, 1116 .born_from = from,
1124 .host = tiles[from].host, 1117 .host = tiles[from].host,
1125 }) orelse { 1118 .borrowed = true,
1126 // A refused wall keeps nothing, so neither may the copies made for 1119 }) orelse return .full;
1127 // it: `birthTile` took ownership only by returning a slot.
1128 alloc.free(session);
1129 alloc.free(label);
1130 return .full;
1131 };
1132 spawnPump(&tiles[at]); 1120 spawnPump(&tiles[at]);
1133 return .{ .moved = at }; 1121 return .{ .moved = at };
1134 } 1122 }