a73x

6fe188fc

fix: review round 1 — an unborrowed birth's copies have one owner

a73x   2026-08-28 19:53

Commit message
fix: review round 1 — an unborrowed birth's copies have one owner

`Birth.borrowed = false` became an ownership claim when the slot's next
reuse started freeing what it was handed: it now says so, and a test named
for it lets the testing allocator judge — a free of a slice it never handed
out aborts, one nobody frees is reported leaked. The beside-wall test was
shipping the unsafe pattern (rodata literals, `borrowed` defaulted) and now
borrows.

`addSessionTile` leaked its two copies on every refusal, three lines from
the rule this branch made load-bearing. `pickBirth` was already right.

`last_focus` was written six times and read nowhere, and its comment named
a `Ctrl-\ l` binding that does not exist.

The e2e's wait after `x` stays a duration: measured, `x` yields exactly ONE
screen clear and a second `expect` in its place times out, so the tile's
departure has nothing to converge on. The comment says that now, and says
why a wait too short fails loudly instead of passing.
docs/decisions.md
Old New
@@ -7342,13 +7342,11 @@ per-host-per-second repaint cannot leave a terminal that never goes quiet.
7342 ## 2026-08-28 — a new tile takes the lowest free digit 7342 ## 2026-08-28 — a new tile takes the lowest free digit
7343 7343
7344 Reuse, not renumbering. `Ctrl-\ 1-9` addresses digits, so a digit an ended 7344 Reuse, not renumbering. `Ctrl-\ 1-9` addresses digits, so a digit an ended
7345 session kept forever pushes later births out of the keyboard's reach, and 7345 session kept forever pushes later births out of the keyboard's reach, while
7346 renumbering the survivors moves a digit the user had learned. `birthTile` 7346 renumbering the survivors moves a digit the user had learned. The daemon
7347 takes the lowest free slot, the daemon already does the same with names 7347 already reuses names, so digit and name come back together.
7348 (`client.nextFreeName`), and digit and name come back together. 7348
7349 7349 A slot is free only once its pump has RETURNED (`Tile.pump_done`): `present`
7350 A slot is free only once its pump has RETURNED (`Tile.pump_done`, stored 7350 alone hands a `*Tile` a returning thread is still reading to a new tile. Its
7351 after the bell that reads `t.shared`): `present` alone hands a `*Tile` a 7351 doorbell pipe stays with it — never closed, so a fresh one per reuse leaks an
7352 returning thread is still reading to a new tile. Its doorbell pipe stays with 7352 fd pair — and `live` becomes the high-water mark, growth counted in `present`.
7353 it — never closed, so a fresh one per reuse leaks an fd pair per birth — and
7354 `live` becomes the high-water mark, so growth is counted in `present`.
src/wallview.zig
Old New
@@ -2367,6 +2367,11 @@ const Birth = struct {
2367 // The poll finds a name in the buffer it is about to reuse and must not 2367 // The poll finds a name in the buffer it is about to reuse and must not
2368 // pay for a copy — nor leave one behind — when the wall refuses the 2368 // pay for a copy — nor leave one behind — when the wall refuses the
2369 // tile; `r.label` is rebuilt here rather than passed. 2369 // tile; `r.label` is rebuilt here rather than passed.
2370 //
2371 // FALSE is an ownership claim, not an optimisation: the tile takes
2372 // `label` and `session` verbatim, and whichever birth takes its digit
2373 // back frees both with `alloc`. Hand it a literal and the abort is that
2374 // later birth, arbitrarily far from the caller that made the mistake.
2370 borrowed: bool = false, 2375 borrowed: bool = false,
2371 }; 2376 };
2372 2377
@@ -2498,7 +2503,10 @@ fn addSessionTile(
2498 // never returns on the success path), which is what lets a pump hold 2503 // never returns on the success path), which is what lets a pump hold
2499 // these slices for as long as it lives. 2504 // these slices for as long as it lives.
2500 const session = alloc.dupe(u8, want) catch return .full; 2505 const session = alloc.dupe(u8, want) catch return .full;
2501 const label = tileLabel(alloc, target, want) catch return .full; 2506 const label = tileLabel(alloc, target, want) catch {
2507 alloc.free(session);
2508 return .full;
2509 };
2502 const at = birthTile(alloc, tiles, present, live, shared, .{ 2510 const at = birthTile(alloc, tiles, present, live, shared, .{
2503 // The offer is inherited from the tile this one grew out of. Same 2511 // The offer is inherited from the tile this one grew out of. Same
2504 // target, so `-A` exposes nothing the user has not already exposed 2512 // target, so `-A` exposes nothing the user has not already exposed
@@ -2511,7 +2519,13 @@ fn addSessionTile(
2511 .creates = true, 2519 .creates = true,
2512 .born_from = from, 2520 .born_from = from,
2513 .host = tiles[from].host, 2521 .host = tiles[from].host,
2514 }) orelse return .full; 2522 }) orelse {
2523 // A refused wall keeps nothing, so neither may the copies made for
2524 // it: `birthTile` took ownership only by returning a slot.
2525 alloc.free(session);
2526 alloc.free(label);
2527 return .full;
2528 };
2515 spawnPump(&tiles[at]); 2529 spawnPump(&tiles[at]);
2516 return .{ .moved = at }; 2530 return .{ .moved = at };
2517 } 2531 }
@@ -3960,9 +3974,6 @@ pub fn run(alloc: std.mem.Allocator, host_specs: []const HostSpec, entry: Entry)
3960 // this chunk, so its room is a whole chunk plus that hold. 3974 // this chunk, so its room is a whole chunk plus that hold.
3961 var mouse_out: [mailbox_max + interact.MouseFilter.max_held]u8 = undefined; 3975 var mouse_out: [mailbox_max + interact.MouseFilter.max_held]u8 = undefined;
3962 var mouse_filter: interact.MouseFilter = .{}; 3976 var mouse_filter: interact.MouseFilter = .{};
3963 // Where `Ctrl-\ l` goes back to. Keyboard-thread state: no pump reads
3964 // it, and no lock guards it, because nothing else writes it.
3965 var last_focus: ?usize = null;
3966 // A vanish sentence deferred to the normal screen: printed after raw 3977 // A vanish sentence deferred to the normal screen: printed after raw
3967 // mode is restored, alongside the exit message below. 3978 // mode is restored, alongside the exit message below.
3968 var vanish_msg: ?[]const u8 = null; 3979 var vanish_msg: ?[]const u8 = null;
@@ -4043,7 +4054,6 @@ pub fn run(alloc: std.mem.Allocator, host_specs: []const HostSpec, entry: Entry)
4043 tiles[z].pending_place, 4054 tiles[z].pending_place,
4044 )) { 4055 )) {
4045 .moved => |to| { 4056 .moved => |to| {
4046 last_focus = z;
4047 focusAnswer(alloc, tiles[0..live], present[0..live], &shared, presentCount(present[0..live]) > before, to); 4057 focusAnswer(alloc, tiles[0..live], present[0..live], &shared, presentCount(present[0..live]) > before, to);
4048 }, 4058 },
4049 .full => setNotice(&shared, "[no room on the wall for another tile]"), 4059 .full => setNotice(&shared, "[no room on the wall for another tile]"),
@@ -4058,7 +4068,6 @@ pub fn run(alloc: std.mem.Allocator, host_specs: []const HostSpec, entry: Entry)
4058 if (endedTile(tiles[0..live], present[0..live], &shared)) |ended| { 4068 if (endedTile(tiles[0..live], present[0..live], &shared)) |ended| {
4059 switch (endAction(tiles, present, live, ended, stdin_open, shared.is_tty)) { 4069 switch (endAction(tiles, present, live, ended, stdin_open, shared.is_tty)) {
4060 .refocus => |to| { 4070 .refocus => |to| {
4061 last_focus = ended;
4062 if (!tiles[to].alive.load(.acquire)) 4071 if (!tiles[to].alive.load(.acquire))
4063 setNotice(&shared, "[focus on a dead tile - no live neighbour]"); 4072 setNotice(&shared, "[focus on a dead tile - no live neighbour]");
4064 setFocus(tiles[0..live], &shared, to); 4073 setFocus(tiles[0..live], &shared, to);
@@ -4336,7 +4345,6 @@ pub fn run(alloc: std.mem.Allocator, host_specs: []const HostSpec, entry: Entry)
4336 if (ev.at > seg_start) 4345 if (ev.at > seg_start)
4337 sendKeys(&tiles[shared.sel], cmd.forward[seg_start..ev.at]); 4346 sendKeys(&tiles[shared.sel], cmd.forward[seg_start..ev.at]);
4338 seg_start = ev.at; 4347 seg_start = ev.at;
4339 last_focus = shared.sel;
4340 setFocus(tiles[0..live], &shared, hit); 4348 setFocus(tiles[0..live], &shared, hit);
4341 } 4349 }
4342 } 4350 }
@@ -4397,7 +4405,6 @@ pub fn run(alloc: std.mem.Allocator, host_specs: []const HostSpec, entry: Entry)
4397 // every session every host has, so asking one daemon for a 4405 // every session every host has, so asking one daemon for a
4398 // ring would walk its slice and skip the rest of the wall. 4406 // ring would walk its slice and skip the rest of the wall.
4399 if (walkTiles(present[0..live], z, cmd.action == .next_session)) |to| { 4407 if (walkTiles(present[0..live], z, cmd.action == .next_session)) |to| {
4400 last_focus = z;
4401 focusAnswer(alloc, tiles[0..live], present[0..live], &shared, false, to); 4408 focusAnswer(alloc, tiles[0..live], present[0..live], &shared, false, to);
4402 } 4409 }
4403 }, 4410 },
@@ -4406,7 +4413,6 @@ pub fn run(alloc: std.mem.Allocator, host_specs: []const HostSpec, entry: Entry)
4406 if (flat) |f| { 4413 if (flat) |f| {
4407 if (layout.neighbor(f, @intCast(z), dirOf(d))) |nb| { 4414 if (layout.neighbor(f, @intCast(z), dirOf(d))) |nb| {
4408 if (nb < live and present[nb] and nb != z) { 4415 if (nb < live and present[nb] and nb != z) {
4409 last_focus = z;
4410 if (shared.fullscreen) 4416 if (shared.fullscreen)
4411 focusAnswer(alloc, tiles[0..live], present[0..live], &shared, false, nb) 4417 focusAnswer(alloc, tiles[0..live], present[0..live], &shared, false, nb)
4412 else 4418 else
@@ -4424,7 +4430,6 @@ pub fn run(alloc: std.mem.Allocator, host_specs: []const HostSpec, entry: Entry)
4424 }, 4430 },
4425 .focus => |idx| { 4431 .focus => |idx| {
4426 if (idx > 0 and idx <= live and present[idx - 1] and idx - 1 != z) { 4432 if (idx > 0 and idx <= live and present[idx - 1] and idx - 1 != z) {
4427 last_focus = z;
4428 if (shared.fullscreen) 4433 if (shared.fullscreen)
4429 focusAnswer(alloc, tiles[0..live], present[0..live], &shared, false, idx - 1) 4434 focusAnswer(alloc, tiles[0..live], present[0..live], &shared, false, idx - 1)
4430 else 4435 else
@@ -5737,9 +5742,15 @@ test "birthTile: a beside wall admits more panes than rows/3" {
5737 .place = .beside_focus, 5742 .place = .beside_focus,
5738 .creates = true, 5743 .creates = true,
5739 .born_from = n - 2, 5744 .born_from = n - 2,
5745 // Borrowed, so the tile gets copies of its own: these are literals,
5746 // and `Birth.borrowed = false` promises the slot's next reuse two
5747 // slices it may free.
5748 .borrowed = true,
5740 }) orelse return error.TestUnexpectedResult; 5749 }) orelse return error.TestUnexpectedResult;
5741 defer std.posix.close(tiles[at].wake_r); 5750 defer std.posix.close(tiles[at].wake_r);
5742 defer std.posix.close(tiles[at].wake_w); 5751 defer std.posix.close(tiles[at].wake_w);
5752 defer alloc.free(tiles[at].r.session);
5753 defer alloc.free(tiles[at].r.label);
5743 try std.testing.expectEqual(@as(usize, n), shared.tree.count()); 5754 try std.testing.expectEqual(@as(usize, n), shared.tree.count());
5744 // Admitted AND habitable: every pane keeps the full height and clears 5755 // Admitted AND habitable: every pane keeps the full height and clears
5745 // the column floor, which is what makes the refusal wrong. 5756 // the column floor, which is what makes the refusal wrong.
@@ -5826,6 +5837,75 @@ test "birthTile: a vanished digit is taken back, and not before its pump returne
5826 try std.testing.expectEqual(doorbell, tiles[1].wake_r); 5837 try std.testing.expectEqual(doorbell, tiles[1].wake_r);
5827 } 5838 }
5828 5839
5840 test "birthTile: the digit's next tile frees the copies an unborrowed birth handed over" {
5841 const alloc = std.testing.allocator;
5842 // `Birth.borrowed = false` hands the tile two slices outright, and the
5843 // birth that takes its digit back is what frees them. The testing
5844 // allocator is the whole oracle: a slice it never handed out aborts the
5845 // free, and one nobody frees is reported leaked — which is what a
5846 // caller passing a literal and a reuse that kept the old copies each
5847 // look like.
5848 var shared = Shared{ .out_fd = -1, .size = .{ .cols = 200, .rows = 24 }, .is_tty = false };
5849 defer shared.tree.deinit();
5850 try shared.tree.addFirst(0);
5851 var tiles: [3]Tile = undefined;
5852 var present = [_]bool{ true, false, false };
5853 var live: usize = 1;
5854 const target: client.Target = .{ .sock = "/tmp/a" };
5855 tiles[0] = .{
5856 .r = .{ .target = target, .label = "--sock /tmp/a#0", .session = "0" },
5857 .rect = .{ .top = 0, .left = 0, .rows = 24, .cols = 200 },
5858 .shared = &shared,
5859 .idx = 0,
5860 .wake_r = -1,
5861 .wake_w = -1,
5862 };
5863 // The chord's road, verbatim: the caller allocates, `birthTile` takes
5864 // the slices as they are.
5865 const handed = struct {
5866 fn birth(
5867 a: std.mem.Allocator,
5868 ts: []Tile,
5869 ps: []bool,
5870 lv: *usize,
5871 sh: *Shared,
5872 tg: client.Target,
5873 name: []const u8,
5874 ) ?usize {
5875 const session = a.dupe(u8, name) catch return null;
5876 const label = tileLabel(a, tg, session) catch {
5877 a.free(session);
5878 return null;
5879 };
5880 return birthTile(a, ts, ps, lv, sh, .{
5881 .r = .{ .target = tg, .label = label, .session = session },
5882 .from = 0,
5883 .place = .beside_focus,
5884 .creates = true,
5885 .born_from = 0,
5886 });
5887 }
5888 }.birth;
5889
5890 const first = handed(alloc, &tiles, &present, &live, &shared, target, "1") orelse
5891 return error.TestUnexpectedResult;
5892 try std.testing.expectEqual(@as(usize, 1), first);
5893 defer if (present[1]) {
5894 alloc.free(tiles[1].r.session);
5895 alloc.free(tiles[1].r.label);
5896 std.posix.close(tiles[1].wake_r);
5897 std.posix.close(tiles[1].wake_w);
5898 };
5899
5900 vanishTile(&tiles, &present, &shared, 1, null);
5901 tiles[1].pump_done.store(true, .release);
5902 // The same digit, so the free under test is one this birth really did.
5903 try std.testing.expectEqual(
5904 @as(?usize, 1),
5905 handed(alloc, &tiles, &present, &live, &shared, target, "2"),
5906 );
5907 }
5908
5829 test "labelText: the state word survives truncation at every width" { 5909 test "labelText: the state word survives truncation at every width" {
5830 var buf: [256]u8 = undefined; 5910 var buf: [256]u8 = undefined;
5831 // A label longer than any bar, so truncation is what is under test and 5911 // A label longer than any bar, so truncation is what is under test and
test/e2e_13_birth.sh
Old New
@@ -581,16 +581,21 @@ ok "a refusal the birth cannot fix backs off instead of spinning"
581 # 1 2 by counting, testing nothing at all. 581 # 1 2 by counting, testing nothing at all.
582 # 582 #
583 # The convergence points are relayout's screen clear — `\x1cc` re-cuts the 583 # The convergence points are relayout's screen clear — `\x1cc` re-cuts the
584 # wall and so does the tile leaving it. `\x1c2` emits none of its own 584 # wall. `\x1c2` emits none of its own (`setFocus` repaints bars, it does not
585 # (`setFocus` repaints bars, it does not clear), which is what makes the 585 # clear), which is what makes the clear after the `x` the ended tile's and
586 # clear after the `x` the dead tile's refocus and not the focus move's. 586 # not the focus move's.
587 # 587 #
588 # The four seconds after that clear are the daemon's LIST, not a guess at 588 # The four seconds after that clear are a DURATION and not a convergence
589 # how long a shell takes to die: `x` leaves a dead tile narrating its exit 589 # point, because there is nothing left to converge on: measured on this
590 # and the tile goes on the next poll a second later. A `c` pressed before 590 # harness, `x` yields exactly ONE clear, and a second `expect \x1b[2J` in
591 # that finds the dying tile still wearing the name the daemon has already 591 # place of this settle times out. The tile's departure is not separately
592 # freed, and merely focuses it — which reads here as `lfd-back` never 592 # announced, so the wait is the daemon's LIST — `x` leaves a dead tile
593 # arriving, never as a green run. 593 # narrating its exit and the tile goes on the next poll a second later.
594 #
595 # A `c` pressed before that finds the dying tile still wearing the name the
596 # daemon has already freed and merely focuses it, which reads here as
597 # `lfd-back` never arriving and as the three assertions below failing —
598 # never as a green run. That is what makes a duration acceptable here.
594 # 599 #
595 # The clear after the LAST `\x1cc` is the other half of the claim: a birth 600 # The clear after the LAST `\x1cc` is the other half of the claim: a birth
596 # that takes a digit back leaves the slot high-water mark where it was, so 601 # that takes a digit back leaves the slot high-water mark where it was, so