From f1a5fab45264b2c1ef4bee0bda2d0cc724300682 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 10 Jul 2026 07:15:57 -0700 Subject: [PATCH 01/12] terminal: keep page serial floor conservative Page generations are not ordered with the page list because splits and in-place replacements insert fresh generations before older live pages. Advancing page_serial_min while pruning could therefore reject live successors and fail PageList integrity checks. Keep the floor as a whole-list invalidation epoch and use the existing pointer-plus-generation membership check for ordinary removals. Add bounded pruning coverage for split and replacement ordering, and verify reset still rejects a stale generation when its node address is reused. --- src/terminal/PageList.zig | 183 +++++++++++++++++++++++++++++++------- 1 file changed, 150 insertions(+), 33 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 1ba939f8f..0eaa4f661 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -365,22 +365,28 @@ pool: MemoryPool, /// The list of pages in the screen. pages: List, -/// A monotonically increasing serial number that is incremented each -/// time a page is allocated or reused as new. The serial is assigned to -/// the Node. +/// The next globally unique page generation for this PageList. A generation +/// is assigned whenever a page is allocated or reused as new. /// /// The serial number can be used to detect whether the page is identical /// to the page that was originally referenced by a pointer. Since we reuse /// and pool memory, pointer stability is not guaranteed, but the serial -/// will always be different for different allocations. +/// will always be different for different page generations. /// /// Developer note: we never do overflow checking on this. If we created /// a new page every second it'd take 584 billion years to overflow. We're /// going to risk it. page_serial: u64, -/// The lowest still valid serial number that could exist. This allows -/// for quick comparisons to find invalid pages in references. +/// A conservative lower bound on live page generations. This is an epoch for +/// whole-list invalidation, not the generation of the first page or the exact +/// minimum live generation. Page generations are not monotonic in list order: +/// replacement and split operations can put a fresh generation before older +/// live pages. +/// +/// A generation below this value is definitely invalid. A generation at or +/// above it is only potentially valid and must still be checked against the +/// live list with `nodeIsValid` before its coordinates are used. page_serial_min: u64, /// Byte size of the raw backing mappings owned by active page nodes. This is @@ -3537,11 +3543,9 @@ pub fn grow(self: *PageList) Allocator.Error!?*List.Node { self.pages.insertAfter(last, first); self.total_rows += 1; - // We also need to reset the serial number. Since this is the only - // place we ever reuse a serial number, we also can safely set - // page_serial_min to be one more than the old serial because we - // only ever prune the oldest pages. - self.page_serial_min = first.serial + 1; + // Reusing the node gives it a fresh generation. Do not advance + // page_serial_min here: generations are not monotonic in list order, + // so older live successors may have lower generations. first.serial = self.page_serial; self.page_serial += 1; @@ -4576,9 +4580,8 @@ pub fn eraseActive( /// and the page becomes underutilized (size < capacity). /// /// Callers must ensure that the erased range only removes pages from -/// the front or back of the linked list, never the middle. Middle-page -/// erasure would create serial gaps that page_serial_min cannot -/// represent, leaving dangling references in consumers such as search. +/// the front or back of the linked list, never the middle. The pin and row +/// accounting in this operation is only defined for those boundary ranges. /// Use the public eraseHistory/eraseActive wrappers which enforce this. fn eraseRows( self: *PageList, @@ -4697,17 +4700,10 @@ fn erasePage(self: *PageList, node: *List.Node) void { // Must not be the final page. assert(node.next != null or node.prev != null); - // We only support erasing from the front or back, never the middle. - // Middle erasure would create serial gaps that page_serial_min can't - // represent. If this ever needs to change, we'll need a more - // sophisticated invalidation mechanism. + // We only support erasing from the front or back, never the middle. The + // public erase operations maintain this contract by construction. assert(node.prev == null or node.next == null); - // If we're erasing the first page, update page_serial_min so that - // any external references holding this page's serial will know it - // has been invalidated. - if (node.prev == null) self.page_serial_min = node.next.?.serial; - // Update any tracked pins to move to the previous or next page. const pin_keys = self.tracked_pins.keys(); for (pin_keys) |p| { @@ -6554,6 +6550,21 @@ fn growColdPagesForTest(self: *PageList, count: usize) !void { } } +fn fillLastPageForTest(self: *PageList) !void { + const last = self.pages.last.?; + while (last.rows() < last.capacity().rows) _ = try self.grow(); +} + +fn expectLivePageSerialsValidForTest(self: *const PageList) !void { + const testing = std.testing; + var node = self.pages.first; + while (node) |live| : (node = live.next) { + try testing.expect(live.serial >= self.page_serial_min); + try testing.expect(live.serial < self.page_serial); + try testing.expect(self.nodeIsValid(live, live.serial)); + } +} + test "PageList Pin rightWrap exact row multiple" { const testing = std.testing; @@ -6968,6 +6979,88 @@ test "PageList incremental compression restarts after prune reuse" { try testing.expectEqual(@as(usize, 1), s.memoryStats().compressed_pages); } +test "PageList repeated bounded pruning after split preserves live serials" { + const testing = std.testing; + + var s = try init( + testing.allocator, + 80, + 24, + 3 * PagePool.item_size, + ); + defer s.deinit(); + + while (s.totalPages() < 3) _ = try s.grow(); + const first = s.pages.first.?; + try s.split(.{ + .node = first, + .y = first.rows() / 2, + .x = 0, + }); + + // The split target has a fresh serial but precedes older successor pages. + // Prune both the old source and then that target to exercise the serial + // floor after list order has become non-monotonic. + for (0..2) |_| { + while (s.pages.last.?.rows() < s.pages.last.?.capacity().rows) { + _ = try s.grow(); + } + _ = try s.grow(); + + try s.expectLivePageSerialsValidForTest(); + } +} + +test "PageList bounded pruning after front replacement preserves live serials" { + const testing = std.testing; + + var s = try init( + testing.allocator, + 80, + 24, + 2 * PagePool.item_size, + ); + defer s.deinit(); + + while (s.totalPages() < 2) _ = try s.grow(); + const old = s.pages.first.?; + const old_serial = old.serial; + const replacement = try s.increaseCapacity(old, null); + try testing.expect(replacement != old); + try testing.expect(!s.nodeIsValid(old, old_serial)); + + try s.fillLastPageForTest(); + _ = try s.grow(); + + try s.expectLivePageSerialsValidForTest(); +} + +test "PageList bounded pruning after middle replacement preserves live serials" { + const testing = std.testing; + + var s = try init( + testing.allocator, + 80, + 24, + 3 * PagePool.item_size, + ); + defer s.deinit(); + + while (s.totalPages() < 3) _ = try s.grow(); + const old = s.pages.first.?.next.?; + const old_serial = old.serial; + const replacement = try s.increaseCapacity(old, null); + try testing.expect(replacement != old); + try testing.expect(!s.nodeIsValid(old, old_serial)); + + // Prune the original first page and then the fresh middle replacement. + for (0..2) |_| { + try s.fillLastPageForTest(); + _ = try s.grow(); + try s.expectLivePageSerialsValidForTest(); + } +} + test "PageList incremental compression restarts after earlier replacement" { const testing = std.testing; @@ -15841,19 +15934,43 @@ test "PageList reset invalidates stale untracked refs even if node memory is reu var s = try init(alloc, 80, 24, null); defer s.deinit(); - const old_serial = s.pages.first.?.serial; - try testing.expect(old_serial >= s.page_serial_min); - try testing.expect(old_serial < s.page_serial); + var stale_nodes: [page_preheat * 4]*List.Node = undefined; + var stale_serials: [stale_nodes.len]u64 = undefined; + var stale_len: usize = 0; + var reused: ?struct { *List.Node, u64 } = null; - s.reset(); + while (stale_len < stale_nodes.len and reused == null) { + const old_node = s.pages.first.?; + const old_serial = old_node.serial; + try testing.expect(old_serial >= s.page_serial_min); + try testing.expect(old_serial < s.page_serial); + stale_nodes[stale_len] = old_node; + stale_serials[stale_len] = old_serial; + stale_len += 1; - // The important safety property is that stale serials are rejected before - // the node pointer is inspected. Reset rebuilds the page list from the - // pools, so old untracked refs may contain node pointers that are no - // longer safe to dereference. + s.reset(); + + const new_node = s.pages.first.?; + for (stale_nodes[0..stale_len], stale_serials[0..stale_len]) |node, serial| { + if (node == new_node) { + reused = .{ node, serial }; + break; + } + } + } + + try testing.expect(reused != null); + const old_node, const old_serial = reused.?; + const new_node = s.pages.first.?; + const new_serial = new_node.serial; + + // Reset advances the epoch before rebuilding from the node pool. Reject + // the stale generation before inspecting its pointer, even when that exact + // address now belongs to a new live generation. + try testing.expectEqual(old_node, new_node); try testing.expect(old_serial < s.page_serial_min); - - const new_serial = s.pages.first.?.serial; + try testing.expect(!s.nodeIsValid(old_node, old_serial)); + try testing.expect(s.nodeIsValid(new_node, new_serial)); try testing.expect(new_serial >= s.page_serial_min); try testing.expect(new_serial < s.page_serial); } From a804e3ac78b21978c3513ccde47901da096b6844 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 10 Jul 2026 07:18:43 -0700 Subject: [PATCH 02/12] terminal: invalidate partially erased page refs Partial history erasure shifted retained rows while preserving the node generation. Cached flattened matches then passed pointer-plus- generation validation and could be tracked with coordinates beyond the shortened page. Renew the generation before reinitializing or shifting a page layout, and mark compression activity so an incremental pass restarts and revisits a restored cold page. The conservative serial floor permits the fresh generation to remain before older live successors without rejecting them. --- src/terminal/PageList.zig | 78 +++++++++++++++++++++++++++++++++- src/terminal/search/screen.zig | 33 ++++++++++++++ 2 files changed, 109 insertions(+), 2 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 0eaa4f661..ea7f00d18 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -365,8 +365,9 @@ pool: MemoryPool, /// The list of pages in the screen. pages: List, -/// The next globally unique page generation for this PageList. A generation -/// is assigned whenever a page is allocated or reused as new. +/// The next globally unique reference generation for this PageList. A +/// generation is assigned whenever a page is allocated, reused as new, or +/// changed in place such that existing page coordinates are no longer stable. /// /// The serial number can be used to detect whether the page is identical /// to the page that was originally referenced by a pointer. Since we reuse @@ -3099,6 +3100,14 @@ pub fn scrollClear(self: *PageList) Allocator.Error!void { for (0..non_empty) |_| _ = try self.grow(); } +/// Renew a live node's generation before changing its coordinate layout in +/// place. This invalidates external pointer-plus-generation references without +/// changing the whole-list invalidation floor. +fn invalidateNodeLayout(self: *PageList, node: *List.Node) void { + node.serial = self.page_serial; + self.page_serial += 1; +} + /// Compact a page to use the minimum required memory for the contents /// it stores. Returns the new node pointer if compaction occurred, or null /// if the page was already compact or compaction would not provide any @@ -4589,6 +4598,7 @@ fn eraseRows( bl_pt: ?point.Point, ) void { defer self.assertIntegrity(); + self.page_compression.markActivity(); // The count of rows that was erased. var erased: usize = 0; @@ -4606,6 +4616,7 @@ fn eraseRows( // page so to handle this we reinit this page, set it to zero // size which will let us grow our active area back. if (chunk.node.next == null and chunk.node.prev == null) { + self.invalidateNodeLayout(chunk.node); const page = chunk.node.page(); erased += page.size.rows; page.reinit(); @@ -4618,6 +4629,10 @@ fn eraseRows( continue; } + // Moving the retained suffix changes the meaning and valid range of + // every captured row coordinate in this node. + self.invalidateNodeLayout(chunk.node); + // We are modifying our chunk so make sure it is in a good state. const page = chunk.node.page(); defer page.assertIntegrity(); @@ -6979,6 +6994,65 @@ test "PageList incremental compression restarts after prune reuse" { try testing.expectEqual(@as(usize, 1), s.memoryStats().compressed_pages); } +test "PageList bounded pruning after partial erase preserves live serials" { + const testing = std.testing; + + var s = try init( + testing.allocator, + 80, + 24, + 2 * PagePool.item_size, + ); + defer s.deinit(); + + while (s.totalPages() < 2) _ = try s.grow(); + const first = s.pages.first.?; + const old_serial = first.serial; + const old_rows = first.rows(); + + s.eraseHistory(.{ .history = .{ .y = 0 } }); + try testing.expectEqual(first, s.pages.first.?); + try testing.expectEqual(old_rows - 1, first.rows()); + try testing.expect(!s.nodeIsValid(first, old_serial)); + + try s.fillLastPageForTest(); + _ = try s.grow(); + try s.expectLivePageSerialsValidForTest(); +} + +test "PageList partial erase restarts compression before continuation" { + const testing = std.testing; + + var s = try init(testing.allocator, 80, 24, null); + defer s.deinit(); + try s.growColdPagesForTest(incremental_compression_max_inspected + 1); + _ = s.compress(.full); + + const first = s.pages.first.?; + try testing.expect(first.isCompressed()); + + var marker = first; + for (1..incremental_compression_max_inspected) |_| marker = marker.next.?; + s.page_compression = .{ + .flags = .{ .verifying = true }, + .last_serial = marker.serial, + .next_serial = s.page_serial, + }; + + const activity = s.page_compression.activity_serial; + s.eraseHistory(.{ .history = .{ .y = 0 } }); + try testing.expect(!first.isCompressed()); + try testing.expect(activity != s.page_compression.activity_serial); + + // The changed generation is before the saved marker, so continuation must + // restart and recompress it instead of reporting verification complete. + try testing.expectEqual( + IncrementalCompressionResult.pending, + s.compress(.incremental), + ); + try testing.expect(first.isCompressed()); +} + test "PageList repeated bounded pruning after split preserves live serials" { const testing = std.testing; diff --git a/src/terminal/search/screen.zig b/src/terminal/search/screen.zig index 448bb98c3..71c6fe184 100644 --- a/src/terminal/search/screen.zig +++ b/src/terminal/search/screen.zig @@ -1616,6 +1616,39 @@ test "select after history compaction ignores replaced results" { try testing.expect(search.selected == null); } +test "select after partial history page erase ignores shifted results" { + const alloc = testing.allocator; + var t: Terminal = try .init(alloc, .{ + .cols = 10, + .rows = 2, + .max_scrollback = std.math.maxInt(usize), + }); + defer t.deinit(alloc); + const list: *PageList = &t.screens.active.pages; + + var stream = t.vtStream(); + defer stream.deinit(); + + const first = list.pages.first.?; + while (first.rows() < first.capacity().rows) stream.nextSlice("\r\n"); + stream.nextSlice("error"); + for (0..list.rows + 1) |_| stream.nextSlice("\r\n"); + try testing.expect(first != list.pages.last.?); + + var search: ScreenSearch = try .init(alloc, t.screens.active, "error"); + defer search.deinit(); + try search.searchAll(); + try testing.expectEqual(0, search.active_results.items.len); + try testing.expectEqual(1, search.history_results.items.len); + + const old_rows = first.rows(); + list.eraseHistory(.{ .history = .{ .y = 0 } }); + try testing.expectEqual(old_rows - 1, first.rows()); + + try testing.expect(!try search.select(.next)); + try testing.expect(search.selected == null); +} + test "screen search no scrollback has no history" { const alloc = testing.allocator; var t: Terminal = try .init(alloc, .{ From 91a63e5287c06c5a9545752028a3afe5ef7b4933 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 10 Jul 2026 07:20:29 -0700 Subject: [PATCH 03/12] terminal: invalidate split source page refs PageList.split moved the source suffix into a fresh target but left the shortened source on its old generation. Cached matches in that suffix could therefore pass validation against the live source pointer and reach pin tracking with invalid coordinates. Renew the source generation only after target cloning succeeds, and mark compression activity so restored history is reconsidered. The conservative floor keeps bounded pruning safe even when the renewed source and fresh target precede older successors. --- src/terminal/PageList.zig | 35 ++++++++++++++++++++++++++++++++++ src/terminal/search/screen.zig | 33 ++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index ea7f00d18..9b0f5838e 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -3252,6 +3252,12 @@ pub fn split( // error handling. It is possible but we haven't written it. errdefer comptime unreachable; + // The source is about to describe a shorter row range. Renew its + // generation only after every failable step has succeeded so a failed + // split leaves existing references valid. + self.invalidateNodeLayout(original_node); + self.page_compression.markActivity(); + // Move any tracked pins from the copied rows for (self.tracked_pins.keys()) |tracked| { if (tracked.node.page() != page or @@ -7053,6 +7059,35 @@ test "PageList partial erase restarts compression before continuation" { try testing.expect(first.isCompressed()); } +test "PageList bounded pruning after split invalidation preserves live serials" { + const testing = std.testing; + + var s = try init( + testing.allocator, + 80, + 24, + 2 * PagePool.item_size, + ); + defer s.deinit(); + + while (s.totalPages() < 2) _ = try s.grow(); + const first = s.pages.first.?; + const old_serial = first.serial; + const activity = s.page_compression.activity_serial; + + try s.split(.{ + .node = first, + .y = first.rows() / 2, + .x = 0, + }); + try testing.expect(!s.nodeIsValid(first, old_serial)); + try testing.expect(activity != s.page_compression.activity_serial); + + try s.fillLastPageForTest(); + _ = try s.grow(); + try s.expectLivePageSerialsValidForTest(); +} + test "PageList repeated bounded pruning after split preserves live serials" { const testing = std.testing; diff --git a/src/terminal/search/screen.zig b/src/terminal/search/screen.zig index 71c6fe184..5bd977240 100644 --- a/src/terminal/search/screen.zig +++ b/src/terminal/search/screen.zig @@ -1649,6 +1649,39 @@ test "select after partial history page erase ignores shifted results" { try testing.expect(search.selected == null); } +test "select after history page split ignores moved results" { + const alloc = testing.allocator; + var t: Terminal = try .init(alloc, .{ + .cols = 10, + .rows = 2, + .max_scrollback = std.math.maxInt(usize), + }); + defer t.deinit(alloc); + const list: *PageList = &t.screens.active.pages; + + var stream = t.vtStream(); + defer stream.deinit(); + + const first = list.pages.first.?; + while (first.rows() < first.capacity().rows) stream.nextSlice("\r\n"); + stream.nextSlice("error"); + for (0..list.rows + 1) |_| stream.nextSlice("\r\n"); + + var search: ScreenSearch = try .init(alloc, t.screens.active, "error"); + defer search.deinit(); + try search.searchAll(); + try testing.expectEqual(1, search.history_results.items.len); + + try list.split(.{ + .node = first, + .y = first.rows() / 2, + .x = 0, + }); + + try testing.expect(!try search.select(.next)); + try testing.expect(search.selected == null); +} + test "screen search no scrollback has no history" { const alloc = testing.allocator; var t: Terminal = try .init(alloc, .{ From 83aada2056a5668a984f0345908487857acfae9a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 10 Jul 2026 07:20:56 -0700 Subject: [PATCH 04/12] terminal/search: preserve serial order in reverse matches Reverse multi-page highlight construction reordered node and row- bound columns but left captured page generations in their original order. Every cross-page result therefore paired each node with another page generation and could be rejected as stale despite remaining live. Reverse the serial column with the other flattened chunk metadata and cover the node-plus-generation pairing in the existing boundary match test. --- src/terminal/search/sliding_window.zig | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/terminal/search/sliding_window.zig b/src/terminal/search/sliding_window.zig index 5c29ce03b..2f858ec85 100644 --- a/src/terminal/search/sliding_window.zig +++ b/src/terminal/search/sliding_window.zig @@ -489,12 +489,14 @@ pub const SlidingWindow = struct { .reverse => { const slice = self.chunk_buf.slice(); const nodes = slice.items(.node); + const serials = slice.items(.serial); const starts = slice.items(.start); const ends = slice.items(.end); if (self.chunk_buf.len > 1) { // Reverse all our chunks. This should be pretty obvious why. std.mem.reverse(*PageList.List.Node, nodes); + std.mem.reverse(u64, serials); std.mem.reverse(size.CellCountInt, starts); std.mem.reverse(size.CellCountInt, ends); @@ -1447,6 +1449,15 @@ test "SlidingWindow two pages match across boundary reversed" { // Search should find a match { const h = w.next().?; + const chunks = h.chunks.slice(); + const nodes = chunks.items(.node); + const serials = chunks.items(.serial); + try testing.expectEqual(2, chunks.len); + try testing.expectEqual(node, nodes[0]); + try testing.expectEqual(node.serial, serials[0]); + try testing.expectEqual(node.next.?, nodes[1]); + try testing.expectEqual(node.next.?.serial, serials[1]); + const sel = h.untracked(); try testing.expectEqual(point.Point{ .active = .{ .x = 76, From c243d89bdf78e5341ca0b7e509d0798bd60c06ab Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 10 Jul 2026 07:21:28 -0700 Subject: [PATCH 05/12] terminal/search: include generations in viewport fingerprints ViewportSearch compared only cached node addresses when deciding whether to reuse its owned search window. An in-place layout change or pool address reuse could therefore make stale text and coordinates appear current. Snapshot each node generation with its pointer and compare only those captured values. This detects a renewed generation at the same address without ever dereferencing cached node pointers. --- src/terminal/search/viewport.zig | 73 +++++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 19 deletions(-) diff --git a/src/terminal/search/viewport.zig b/src/terminal/search/viewport.zig index d8dbb9429..0493c0d6f 100644 --- a/src/terminal/search/viewport.zig +++ b/src/terminal/search/viewport.zig @@ -103,9 +103,9 @@ pub const ViewportSearch = struct { // If our viewport contains the start or end of the active area, // we are in the active area. We purposely do this first // because our viewport is always larger than the active area. - for (old.nodes) |node| { - if (node == active_tl.node) break :match; - if (node == active_br.node) break :match; + for (old.entries) |entry| { + if (entry.node == active_tl.node) break :match; + if (entry.node == active_br.node) break :match; } } @@ -135,7 +135,7 @@ pub const ViewportSearch = struct { // exists) so we can cover the overlap. We only do this for the // soft-wrapped prior pages. const overlap_len = self.window.needle.len -| 1; - var node_ = fingerprint.nodes[0].prev; + var node_ = fingerprint.entries[0].node.prev; var added: usize = 0; while (node_) |node| : (node_ = node.prev) { // We could be more accurate here and count bytes since the @@ -149,13 +149,13 @@ pub const ViewportSearch = struct { // We can use our fingerprint nodes to initialize our sliding // window, since we already traversed the viewport once. var end_append: SlidingWindow.AppendResult = undefined; - for (fingerprint.nodes) |node| { - end_append = try self.window.append(node); + for (fingerprint.entries) |entry| { + end_append = try self.window.append(entry.node); } // Add any trailing overlap as well. trailing: { - const end: *PageList.List.Node = fingerprint.nodes[fingerprint.nodes.len - 1]; + const end: *PageList.List.Node = fingerprint.entries[fingerprint.entries.len - 1].node; if (!end_append.last_row_wrapped) break :trailing; node_ = end.next; @@ -181,14 +181,18 @@ pub const ViewportSearch = struct { /// Viewport fingerprint so we can detect when the viewport moves. const Fingerprint = struct { - /// The nodes that make up the viewport. We need to flatten this - /// to a single list because we can't safely traverse the cached values - /// because the page nodes may be invalid. All that is safe is comparing - /// the actual pointer values. - nodes: []const *PageList.List.Node, + /// The node addresses and generations that make up the viewport. We + /// can't safely dereference cached node pointers because the nodes may + /// be invalid, so equality only compares these captured values. + entries: []const Entry, + + const Entry = struct { + node: *PageList.List.Node, + serial: u64, + }; pub fn init(alloc: Allocator, pages: *PageList) Allocator.Error!Fingerprint { - var list: std.ArrayList(*PageList.List.Node) = .empty; + var list: std.ArrayList(Entry) = .empty; defer list.deinit(alloc); // Get our viewport area. Bottom right of a viewport can never @@ -197,18 +201,22 @@ pub const ViewportSearch = struct { const br = pages.getBottomRight(.viewport).?; var it = tl.pageIterator(.right_down, br); - while (it.next()) |chunk| try list.append(alloc, chunk.node); - return .{ .nodes = try list.toOwnedSlice(alloc) }; + while (it.next()) |chunk| try list.append(alloc, .{ + .node = chunk.node, + .serial = chunk.node.serial, + }); + return .{ .entries = try list.toOwnedSlice(alloc) }; } pub fn deinit(self: *Fingerprint, alloc: Allocator) void { - alloc.free(self.nodes); + alloc.free(self.entries); } pub fn eql(self: Fingerprint, other: Fingerprint) bool { - if (self.nodes.len != other.nodes.len) return false; - for (self.nodes, other.nodes) |a, b| { - if (a != b) return false; + if (self.entries.len != other.entries.len) return false; + for (self.entries, other.entries) |a, b| { + if (a.node != b.node) return false; + if (a.serial != b.serial) return false; } return true; } @@ -258,6 +266,33 @@ test "simple search" { try testing.expect(search.next() == null); } +test "fingerprint detects node generation changes" { + const alloc = testing.allocator; + var t: Terminal = try .init(alloc, .{ .cols = 10, .rows = 10 }); + defer t.deinit(alloc); + + var search: ViewportSearch = try .init(alloc, "needle"); + defer search.deinit(); + search.active_dirty = false; + + try testing.expect(try search.update(&t.screens.active.pages)); + try testing.expect(!try search.update(&t.screens.active.pages)); + + const old_entry = search.fingerprint.?.entries[0]; + const node = t.screens.active.pages.pages.first.?; + try testing.expectEqual(node, old_entry.node); + const original_serial = node.serial; + defer node.serial = original_serial; + node.serial += 1; + + // The viewport still contains the same pointer, but its generation has + // changed, so the cached window must be rebuilt. + try testing.expect(try search.update(&t.screens.active.pages)); + const new_entry = search.fingerprint.?.entries[0]; + try testing.expectEqual(old_entry.node, new_entry.node); + try testing.expect(old_entry.serial != new_entry.serial); +} + test "clear screen and search" { const alloc = testing.allocator; var t: Terminal = try .init(alloc, .{ .cols = 10, .rows = 10 }); From c9ad708ef77a953a995d59725d11fcf14f8cd74c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 10 Jul 2026 07:21:55 -0700 Subject: [PATCH 06/12] terminal: validate flattened highlight generations RenderState applied flattened highlights after releasing the terminal lock and matched them to copied rows by node address alone. A recycled address could therefore make a stale asynchronous highlight decorate unrelated content. Capture the live node generation with each render row and require both the pointer and copied generation to match. Validation remains lock- free and never dereferences a potentially stale node. --- src/terminal/render.zig | 125 ++++++++++++++++++++++++++++++++++------ 1 file changed, 108 insertions(+), 17 deletions(-) diff --git a/src/terminal/render.zig b/src/terminal/render.zig index 01ded1d6b..04f0c9679 100644 --- a/src/terminal/render.zig +++ b/src/terminal/render.zig @@ -209,10 +209,16 @@ pub const RenderState = struct { /// change often. arena: ArenaAllocator.State, - /// The page pin. This is not safe to read unless you can guarantee - /// the terminal state hasn't changed since the last `update` call. + /// The page pin. Its copied values may be compared, but its node must + /// not be dereferenced unless the terminal state is protected from + /// changes since the last `update` call. pin: PageList.Pin, + /// The page node generation captured alongside `pin`. This lets + /// consumers validate the pin without dereferencing its node after + /// the terminal lock has been released. + serial: u64, + /// Raw row data. raw: page.Row, @@ -452,6 +458,7 @@ pub const RenderState = struct { row_data.set(y, .{ .arena = .{}, .pin = undefined, + .serial = undefined, .raw = undefined, .cells = .empty, .dirty = true, @@ -477,6 +484,7 @@ pub const RenderState = struct { const row_data = self.row_data.slice(); const row_arenas = row_data.items(.arena); const row_pins = row_data.items(.pin); + const row_serials = row_data.items(.serial); const row_rows = row_data.items(.raw); const row_cells = row_data.items(.cells); const row_sels = row_data.items(.selection); @@ -511,6 +519,7 @@ pub const RenderState = struct { while (y < self.rows) { const chunk = page_it.next() orelse break; const node = chunk.node; + const node_serial = node.serial; const p: *page.Page = node.page(); // The number of rows we consume from this chunk. The chunk @@ -552,25 +561,34 @@ pub const RenderState = struct { const page_rows: []page.Row = p.rows.ptr(p.memory)[chunk.start..][0..take]; assert(p.size.cols == self.cols); - // Store our pins. We have to store these even for rows that - // aren't dirty because dirty is only a renderer optimization; - // it doesn't apply to memory movement. This lets us remap any - // cell pins back to an exact entry in our RenderState. + // Store our pins and their node generations. We have to store + // these even for rows that aren't dirty because dirty is only a + // renderer optimization; it doesn't apply to memory movement. + // This lets us remap any cell pins back to an exact entry in our + // RenderState and validate them later without dereferencing a + // potentially stale node. // - // We can skip the writes when the pins are unchanged: if we're - // not redrawing, every pin was stored by a prior update (row - // count changes force a redraw). Within a single update a node - // appears at most once and its stored pins have consecutive y - // values, so if the first and last pins of this chunk's range - // already match then every pin in between matches too. + // We can skip the writes when the pins and serials are unchanged: + // if we're not redrawing, every value was stored by a prior update + // (row count changes force a redraw). Within a single update a + // node appears at most once and its stored pins have consecutive + // y values, so if the first and last entries of this chunk's range + // already match then every entry in between matches too. if (redraw or row_pins[y].node != node or row_pins[y].y != chunk.start or + row_serials[y] != node_serial or row_pins[y + take - 1].node != node or - row_pins[y + take - 1].y != chunk.start + take - 1) + row_pins[y + take - 1].y != chunk.start + take - 1 or + row_serials[y + take - 1] != node_serial) { - for (row_pins[y..][0..take], chunk.start..) |*pin, py| { + for ( + row_pins[y..][0..take], + row_serials[y..][0..take], + chunk.start.., + ) |*pin, *serial, py| { pin.* = .{ .node = node, .y = @intCast(py) }; + serial.* = node_serial; } } @@ -763,22 +781,28 @@ pub const RenderState = struct { const row_arenas = row_data.items(.arena); const row_dirties = row_data.items(.dirty); const row_pins = row_data.items(.pin); + const row_serials = row_data.items(.serial); const row_highlights_slice = row_data.items(.highlights); for ( row_arenas, row_pins, + row_serials, row_highlights_slice, row_dirties, - ) |*row_arena, row_pin, *row_highlights, *dirty| { + ) |*row_arena, row_pin, row_serial, *row_highlights, *dirty| { for (hls) |hl| { const chunks_slice = hl.chunks.slice(); const nodes = chunks_slice.items(.node); + const serials = chunks_slice.items(.serial); const starts = chunks_slice.items(.start); const ends = chunks_slice.items(.end); for (0.., nodes) |i, node| { - // If this node doesn't match or we're not within - // the row range, skip it. + // If this node generation doesn't match or we're not + // within the row range, skip it. Both serials are copied + // values, so this never dereferences a node outside the + // terminal lock. if (node != row_pin.node or + serials[i] != row_serial or row_pin.y < starts[i] or row_pin.y >= ends[i]) continue; @@ -2040,6 +2064,73 @@ test "linkCells with invalid viewport point" { } } +test "flattened highlights require matching page serial" { + const testing = std.testing; + const alloc = testing.allocator; + + var t = try Terminal.init(alloc, .{ + .cols = 10, + .rows = 3, + }); + defer t.deinit(alloc); + + // Capture the live generation while terminal-owned state is in scope so + // we can also verify beginUpdate copies it into the render row. + const live_pin = t.screens.active.pages.getTopLeft(.viewport); + const live_serial = live_pin.node.serial; + + var state: RenderState = .empty; + defer state.deinit(alloc); + try state.update(alloc, &t); + + const pin: PageList.Pin = pin: { + const row_data = state.row_data.slice(); + @memset(row_data.items(.dirty), false); + state.dirty = .false; + break :pin row_data.items(.pin)[0]; + }; + const row_serial = state.row_data.items(.serial)[0]; + try testing.expectEqual(live_pin.node, pin.node); + try testing.expectEqual(live_serial, row_serial); + + // Use the exact node pointer and row captured by the render state, but a + // different generation. A reused node address must not make this stale + // flattened highlight match. + var hl: highlight.Flattened = .{ + .chunks = .empty, + .top_x = 2, + .bot_x = 4, + }; + defer hl.deinit(alloc); + try hl.chunks.append(alloc, .{ + .node = pin.node, + .serial = live_serial ^ 1, + .start = pin.y, + .end = pin.y + 1, + }); + + try state.updateHighlightsFlattened(alloc, 42, &.{hl}); + { + const row_data = state.row_data.slice(); + try testing.expectEqual(0, row_data.items(.highlights)[0].items.len); + try testing.expect(!row_data.items(.dirty)[0]); + try testing.expectEqual(.false, state.dirty); + } + + // The same chunk is accepted once its copied serial also matches. + hl.chunks.items(.serial)[0] = live_serial; + try state.updateHighlightsFlattened(alloc, 42, &.{hl}); + { + const row_data = state.row_data.slice(); + const row_highlights = row_data.items(.highlights)[0].items; + try testing.expectEqual(1, row_highlights.len); + try testing.expectEqual(42, row_highlights[0].tag); + try testing.expectEqual([2]size.CellCountInt{ 2, 4 }, row_highlights[0].range); + try testing.expect(row_data.items(.dirty)[0]); + try testing.expectEqual(.partial, state.dirty); + } +} + test "dirty row resets highlights" { const testing = std.testing; const alloc = testing.allocator; From a89c6133c5be223def8b6ab6c1b9b27f0ba5db41 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 10 Jul 2026 07:24:31 -0700 Subject: [PATCH 07/12] terminal/search: validate history before reload ScreenSearch reloaded active state before pruning stale flattened history. Reload could compare a shifted tracked selection against cached page coordinates and panic before the later validation step ran. Prune history immediately after dimension reconciliation in reloadActive, before any cached coordinate is inspected or converted to a pin. Selection now relies on that ordering and avoids a redundant second prune. --- src/terminal/search/screen.zig | 48 +++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/src/terminal/search/screen.zig b/src/terminal/search/screen.zig index 5bd977240..dc683c9e4 100644 --- a/src/terminal/search/screen.zig +++ b/src/terminal/search/screen.zig @@ -457,6 +457,10 @@ pub const ScreenSearch = struct { // searchers: column reflow may have freed every node they reference. if (try self.resetIfDimensionsChanged()) return; + // Validate flattened history before any coordinate is converted back + // into a pin or compared with a tracked selection below. + self.pruneHistory(); + const tw = reloadActive_tw; // If our selection pin became garbage it means we scrolled off @@ -766,11 +770,9 @@ pub const ScreenSearch = struct { // selected result so no pre-resize pointer is ever dereferenced. _ = try self.resetIfDimensionsChanged(); - // All selection requires valid pins so we prune history and - // reload our active area immediately. This ensures all search - // results point to valid nodes. + // All selection requires valid pins, so reload immediately. Reload + // validates flattened history before inspecting cached coordinates. try self.reloadActive(); - self.pruneHistory(); return switch (to) { .next => try self.selectNext(), @@ -1649,6 +1651,44 @@ test "select after partial history page erase ignores shifted results" { try testing.expect(search.selected == null); } +test "reload after partial history page erase drops shifted selection first" { + const alloc = testing.allocator; + var t: Terminal = try .init(alloc, .{ + .cols = 10, + .rows = 2, + .max_scrollback = std.math.maxInt(usize), + }); + defer t.deinit(alloc); + const list: *PageList = &t.screens.active.pages; + + var stream = t.vtStream(); + defer stream.deinit(); + + const first = list.pages.first.?; + while (first.rows() < first.capacity().rows) stream.nextSlice("\r\n"); + stream.nextSlice("error"); + for (0..list.rows + 1) |_| stream.nextSlice("\r\n"); + + var search: ScreenSearch = try .init(alloc, t.screens.active, "error"); + defer search.deinit(); + try search.searchAll(); + try testing.expect(try search.select(.next)); + try testing.expect(search.selected != null); + + list.eraseHistory(.{ .history = .{ .y = 0 } }); + + // Advance the active boundary into another node and add matches so + // reloadActive updates the history result offsets. It must discard the + // stale selected result before comparing any of its captured coordinates. + const active_node = list.getTopLeft(.active).node; + while (list.getTopLeft(.active).node == active_node) { + stream.nextSlice("error\r\n"); + } + + try search.reloadActive(); + try testing.expect(search.selected == null); +} + test "select after history page split ignores moved results" { const alloc = testing.allocator; var t: Terminal = try .init(alloc, .{ From d0a26d1460cfb55e8ce8c62bdb5ad3b883e2577d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 10 Jul 2026 07:26:00 -0700 Subject: [PATCH 08/12] terminal: reschedule compression after page replacement Compaction and capacity growth publish fresh page generations, so an active incremental traversal can detect them. They did not update the separate activity token, however, and a completed compressor could remain idle after either operation restored or replaced a cold page. Mark successful replacements as compression activity without resetting the exact continuation marker. The next scheduled step can retain valid progress or restart through the existing generation checks. --- src/terminal/PageList.zig | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 9b0f5838e..b09983b57 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -3179,6 +3179,7 @@ pub fn compact(self: *PageList, node: *List.Node) Allocator.Error!?*List.Node { self.pages.insertBefore(node, new_node); self.pages.remove(node); self.destroyNode(node); + self.page_compression.markActivity(); new_page.assertIntegrity(); return new_node; @@ -3722,6 +3723,7 @@ pub fn increaseCapacity( self.pages.insertBefore(node, new_node); self.pages.remove(node); self.destroyNode(node); + self.page_compression.markActivity(); new_page.assertIntegrity(); return new_node; @@ -6765,6 +6767,34 @@ test "PageList owns incremental compression state" { ); } +test "PageList replacements preserve compression continuation and mark activity" { + const testing = std.testing; + + var s = try init(testing.allocator, 80, 24, null); + defer s.deinit(); + + const state: IncrementalCompressionState = .{ + .flags = .{ .verifying = true }, + .activity_serial = 42, + .last_serial = 7, + .next_serial = 8, + }; + const expected: IncrementalCompressionState = .{ + .flags = .{ .verifying = true }, + .activity_serial = 43, + .last_serial = 7, + .next_serial = 8, + }; + + s.page_compression = state; + const replacement = try s.increaseCapacity(s.pages.first.?, null); + try testing.expectEqual(expected, s.page_compression); + + s.page_compression = state; + _ = (try s.compact(replacement)).?; + try testing.expectEqual(expected, s.page_compression); +} + test "PageList incremental compression bounds inspected pages" { const testing = std.testing; From dd956f37cc8154c6c17d2290c11070b472237bc6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 10 Jul 2026 07:28:34 -0700 Subject: [PATCH 09/12] terminal: renew generations for shifted rows Single-row erasure and trailing blank-row trimming can remap or remove stored row coordinates while retaining the same node. External pointer- plus-generation references could therefore survive these less common PageList mutation paths. Renew each affected node inside the traversal that already changes it, and signal compression activity only when row erasure begins in history. This adds constant work per touched page without another list scan or normal append cost. --- src/terminal/PageList.zig | 62 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index b09983b57..e735ee242 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -2729,6 +2729,7 @@ fn trimTrailingBlankRows( max: size.CellCountInt, ) size.CellCountInt { var trimmed: size.CellCountInt = 0; + var invalidated_node: ?*List.Node = null; const bl_pin = self.getBottomRight(.screen).?; var it = bl_pin.rowIterator(.left_up, null); while (it.next()) |row_pin| { @@ -2750,6 +2751,10 @@ fn trimTrailingBlankRows( // No text, we can trim this row. Because it has // no text we can also be sure it has no styling // so we don't need to worry about memory. + if (row_pin.node.rows() > 1 and invalidated_node != row_pin.node) { + self.invalidateNodeLayout(row_pin.node); + invalidated_node = row_pin.node; + } row_pin.node.page().size.rows -= 1; if (row_pin.node.page().size.rows == 0) { self.erasePage(row_pin.node); @@ -4293,9 +4298,12 @@ pub fn eraseRow( var page = node.page(); var rows = page.rows.ptr(page.memory.ptr); + if (!self.pinIsActive(pn)) self.page_compression.markActivity(); + // In order to move the following rows up we rotate the rows array by 1. // The rotate operation turns e.g. [ 0 1 2 3 ] in to [ 1 2 3 0 ], which // works perfectly to move all of our elements where they belong. + self.invalidateNodeLayout(node); fastmem.rotateOnce(Row, rows[pn.y..node.rows()]); // We adjust the tracked pins in this page, moving up any that were below @@ -4347,6 +4355,7 @@ pub fn eraseRow( page = next_page; rows = next_rows; + self.invalidateNodeLayout(node); fastmem.rotateOnce(Row, rows[0..node.rows()]); // Mark the whole page as dirty. @@ -4395,10 +4404,13 @@ pub fn eraseRowBounded( var page = node.page(); var rows = page.rows.ptr(page.memory.ptr); + if (!self.pinIsActive(pn)) self.page_compression.markActivity(); + // If the row limit is less than the remaining rows before the end of the // page, then we clear the row, rotate it to the end of the boundary limit // and update our pins. if (node.rows() - pn.y > limit) { + self.invalidateNodeLayout(node); page.clearCells(&rows[pn.y], 0, node.cols()); fastmem.rotateOnce(Row, rows[pn.y..][0 .. limit + 1]); @@ -4441,6 +4453,7 @@ pub fn eraseRowBounded( return; } + self.invalidateNodeLayout(node); fastmem.rotateOnce(Row, rows[pn.y..node.rows()]); // Mark the whole page as dirty. @@ -4501,6 +4514,7 @@ pub fn eraseRowBounded( // The logic here is very similar to the one before the loop. const shifted_limit = limit - shifted; if (node.rows() > shifted_limit) { + self.invalidateNodeLayout(node); page.clearCells(&rows[0], 0, node.cols()); fastmem.rotateOnce(Row, rows[0 .. shifted_limit + 1]); @@ -4536,6 +4550,7 @@ pub fn eraseRowBounded( return; } + self.invalidateNodeLayout(node); fastmem.rotateOnce(Row, rows[0..node.rows()]); // Mark the whole page as dirty. @@ -9671,6 +9686,53 @@ test "PageList eraseRowBounded invalidates viewport offset cache" { }, s.scrollbar()); } +test "PageList row erasure renews affected page generations" { + const testing = std.testing; + + var s = try init(testing.allocator, 80, 24, null); + defer s.deinit(); + while (s.totalPages() < 2) _ = try s.grow(); + + const first = s.pages.first.?; + const second = first.next.?; + var first_serial = first.serial; + var second_serial = second.serial; + var activity = s.page_compression.activity_serial; + + try s.eraseRow(.{ .history = .{ .y = 0 } }); + try testing.expect(!s.nodeIsValid(first, first_serial)); + try testing.expect(!s.nodeIsValid(second, second_serial)); + try testing.expect(activity != s.page_compression.activity_serial); + + first_serial = first.serial; + second_serial = second.serial; + activity = s.page_compression.activity_serial; + try s.eraseRowBounded( + .{ .history = .{ .y = 0 } }, + first.rows() + 1, + ); + try testing.expect(!s.nodeIsValid(first, first_serial)); + try testing.expect(!s.nodeIsValid(second, second_serial)); + try testing.expect(activity != s.page_compression.activity_serial); +} + +test "PageList trailing row truncation renews page generation" { + const testing = std.testing; + + var s = try init(testing.allocator, 80, 24, null); + defer s.deinit(); + + const node = s.pages.last.?; + const old_serial = node.serial; + const trimmed = s.trimTrailingBlankRows(1); + s.total_rows -= trimmed; + try testing.expectEqual(@as(size.CellCountInt, 1), trimmed); + try testing.expect(!s.nodeIsValid(node, old_serial)); + + _ = try s.grow(); + try s.expectLivePageSerialsValidForTest(); +} + test "PageList eraseRowBounded multi-page invalidates viewport offset cache" { const testing = std.testing; const alloc = testing.allocator; From f6d9a582e41579660c0b491ded1f1970b851f7f3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 10 Jul 2026 08:18:55 -0700 Subject: [PATCH 10/12] terminal: rename page serial floor as epoch page_serial_min no longer represented the minimum live page serial. Its name still suggested an ordering relationship and obscured the remaining reset-only invalidation behavior. Rename the field to page_serial_epoch and document its O(1) rejection and ScreenSearch bulk-pruning utility. Explicitly verify that ordinary bounded pruning does not begin a new whole-list epoch. --- src/terminal/PageList.zig | 137 +++++++++++++++++++++++---------- src/terminal/search/screen.zig | 16 ++-- 2 files changed, 106 insertions(+), 47 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index e735ee242..6a5d0d251 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -379,16 +379,17 @@ pages: List, /// going to risk it. page_serial: u64, -/// A conservative lower bound on live page generations. This is an epoch for -/// whole-list invalidation, not the generation of the first page or the exact -/// minimum live generation. Page generations are not monotonic in list order: -/// replacement and split operations can put a fresh generation before older -/// live pages. +/// The first serial in the current whole-list validity epoch. Only `reset` +/// advances this value, immediately before rebuilding every page. It is not +/// the generation of the first page or the exact minimum live generation. +/// Page generations are not monotonic in list order: replacement and split +/// operations can put a fresh generation before older live pages. /// -/// A generation below this value is definitely invalid. A generation at or -/// above it is only potentially valid and must still be checked against the -/// live list with `nodeIsValid` before its coordinates are used. -page_serial_min: u64, +/// A generation below this epoch is definitely invalid, allowing O(1) +/// rejection in `nodeIsValid` and bulk removal of pre-reset search results. +/// A generation at or above it is only potentially valid and must still be +/// checked against the live list before its coordinates are used. +page_serial_epoch: u64, /// Byte size of the raw backing mappings owned by active page nodes. This is /// logical scrollback accounting and does not change while a mapping is @@ -660,7 +661,7 @@ pub fn init( .pool = pool, .pages = page_list, .page_serial = page_serial, - .page_serial_min = 0, + .page_serial_epoch = 0, .page_size = page_size, .explicit_max_size = max_size orelse std.math.maxInt(usize), .min_max_size = min_max_size, @@ -817,12 +818,11 @@ fn verifyIntegrity(self: *const PageList) IntegrityError!void { actual_total += node.rows(); node_ = node.next; - // While doing this traversal, verify no node has a serial - // number lower than our min. - if (node.serial < self.page_serial_min) { + // Every live node must belong to the current validity epoch. + if (node.serial < self.page_serial_epoch) { log.warn( - "PageList integrity violation: page serial too low serial={} min={}", - .{ node.serial, self.page_serial_min }, + "PageList integrity violation: page serial predates epoch serial={} epoch={}", + .{ node.serial, self.page_serial_epoch }, ); return IntegrityError.PageSerialInvalid; } @@ -923,10 +923,10 @@ pub fn reset(self: *PageList) void { // Reset discards all scrollback, so there is nothing left to compress. self.page_compression.reset(); - // Invalidate all external page refs to the previous list. The reset below - // rebuilds the page list from the pools, so old untracked refs must be - // rejected before any validation attempts to inspect their node pointers. - self.page_serial_min = self.page_serial; + // Begin a new whole-list validity epoch before rebuilding from the pools. + // Every old reference now has a serial below the epoch and can be rejected + // in O(1), even if the node pool later reuses its pointer address. + self.page_serial_epoch = self.page_serial; // We need enough pages/nodes to keep our active area. This should // never fail since we by definition have allocated a page already @@ -1156,7 +1156,7 @@ pub fn clone( .pool = pool, .pages = page_list, .page_serial = page_serial, - .page_serial_min = 0, + .page_serial_epoch = 0, .page_size = page_size, .explicit_max_size = self.explicit_max_size, .min_max_size = self.min_max_size, @@ -2752,6 +2752,7 @@ fn trimTrailingBlankRows( // no text we can also be sure it has no styling // so we don't need to worry about memory. if (row_pin.node.rows() > 1 and invalidated_node != row_pin.node) { + // Shrinking a retained page changes its valid row-coordinate range. self.invalidateNodeLayout(row_pin.node); invalidated_node = row_pin.node; } @@ -3105,9 +3106,42 @@ pub fn scrollClear(self: *PageList) Allocator.Error!void { for (0..non_empty) |_| _ = try self.grow(); } -/// Renew a live node's generation before changing its coordinate layout in -/// place. This invalidates external pointer-plus-generation references without -/// changing the whole-list invalidation floor. +/// Give a live node a new generation before changing its coordinate layout in +/// place. +/// +/// This must be called when a node remains at the same address and stays in the +/// list, but a mutation changes which logical row a `(node, y)` coordinate +/// identifies or whether that coordinate is still in range. Examples include +/// rotating rows after an erase, truncating a page's row range, reinitializing +/// the sole remaining page, or shortening the source page of a split. Without +/// a new generation, a cached pointer, serial, and coordinate could still pass +/// `nodeIsValid` while referring to a different row than it originally did. +/// +/// The caller must pass a node which is currently live in this PageList. Call +/// this at the point the operation commits to changing the layout and before +/// the first such change. When an operation is intended to be atomic, finish +/// its failable preparation first so a failed operation does not needlessly +/// invalidate references. One call per affected node is sufficient when an +/// operation performs several layout changes without exposing intermediate +/// references. +/// +/// This helper is only needed for in-place changes. Removing or replacing a +/// node already invalidates old references through the live-list check in +/// `nodeIsValid`, and newly allocated or reused nodes receive a fresh serial as +/// part of their initialization. Ordinary cell or style changes which preserve +/// the meaning of page coordinates do not require a new generation solely for +/// that reason. +/// +/// The only state changed here is `node.serial` and the next-generation counter +/// `page_serial`. Consequently, every previously captured pointer-plus-serial +/// pair for this node becomes invalid while new references can capture the new +/// generation. This does not advance `page_serial_epoch`: only `reset` starts a +/// new whole-list validity epoch. +/// +/// This also does not mutate the page, update tracked pins or viewport state, +/// adjust row or memory accounting, mark cells dirty, or notify incremental +/// compression. The surrounding operation remains responsible for all such +/// bookkeeping required by its layout change. fn invalidateNodeLayout(self: *PageList, node: *List.Node) void { node.serial = self.page_serial; self.page_serial += 1; @@ -3258,9 +3292,7 @@ pub fn split( // error handling. It is possible but we haven't written it. errdefer comptime unreachable; - // The source is about to describe a shorter row range. Renew its - // generation only after every failable step has succeeded so a failed - // split leaves existing references valid. + // Failable split work is complete; shortening the source changes its row range. self.invalidateNodeLayout(original_node); self.page_compression.markActivity(); @@ -3564,9 +3596,10 @@ pub fn grow(self: *PageList) Allocator.Error!?*List.Node { self.pages.insertAfter(last, first); self.total_rows += 1; - // Reusing the node gives it a fresh generation. Do not advance - // page_serial_min here: generations are not monotonic in list order, - // so older live successors may have lower generations. + // Reusing the node gives it a fresh generation. Do not begin a new + // page_serial_epoch here: generations are not monotonic in list order, + // so older live successors may have lower generations. The epoch only + // advances when reset invalidates the entire list. first.serial = self.page_serial; self.page_serial += 1; @@ -4298,11 +4331,15 @@ pub fn eraseRow( var page = node.page(); var rows = page.rows.ptr(page.memory.ptr); - if (!self.pinIsActive(pn)) self.page_compression.markActivity(); + // Erasing history may restore compressed pages. Mark unconditionally + // because incrementing the activity token is cheaper than locating the + // active boundary. + self.page_compression.markActivity(); // In order to move the following rows up we rotate the rows array by 1. // The rotate operation turns e.g. [ 0 1 2 3 ] in to [ 1 2 3 0 ], which // works perfectly to move all of our elements where they belong. + // Rotating rows changes which logical row cached coordinates identify. self.invalidateNodeLayout(node); fastmem.rotateOnce(Row, rows[pn.y..node.rows()]); @@ -4355,6 +4392,7 @@ pub fn eraseRow( page = next_page; rows = next_rows; + // Rotating this page moves every cached row coordinate up by one. self.invalidateNodeLayout(node); fastmem.rotateOnce(Row, rows[0..node.rows()]); @@ -4404,12 +4442,16 @@ pub fn eraseRowBounded( var page = node.page(); var rows = page.rows.ptr(page.memory.ptr); - if (!self.pinIsActive(pn)) self.page_compression.markActivity(); + // Erasing history may restore compressed pages. Mark unconditionally + // because incrementing the activity token is cheaper than locating the + // active boundary. + self.page_compression.markActivity(); // If the row limit is less than the remaining rows before the end of the // page, then we clear the row, rotate it to the end of the boundary limit // and update our pins. if (node.rows() - pn.y > limit) { + // Rotating this bounded region changes its cached row coordinates. self.invalidateNodeLayout(node); page.clearCells(&rows[pn.y], 0, node.cols()); fastmem.rotateOnce(Row, rows[pn.y..][0 .. limit + 1]); @@ -4453,6 +4495,7 @@ pub fn eraseRowBounded( return; } + // Rotating this suffix changes which logical row its coordinates identify. self.invalidateNodeLayout(node); fastmem.rotateOnce(Row, rows[pn.y..node.rows()]); @@ -4514,6 +4557,7 @@ pub fn eraseRowBounded( // The logic here is very similar to the one before the loop. const shifted_limit = limit - shifted; if (node.rows() > shifted_limit) { + // Rotating this bounded prefix changes its cached row coordinates. self.invalidateNodeLayout(node); page.clearCells(&rows[0], 0, node.cols()); fastmem.rotateOnce(Row, rows[0 .. shifted_limit + 1]); @@ -4550,6 +4594,7 @@ pub fn eraseRowBounded( return; } + // Rotating the whole page moves every cached row coordinate up by one. self.invalidateNodeLayout(node); fastmem.rotateOnce(Row, rows[0..node.rows()]); @@ -4639,6 +4684,7 @@ fn eraseRows( // page so to handle this we reinit this page, set it to zero // size which will let us grow our active area back. if (chunk.node.next == null and chunk.node.prev == null) { + // Reinitializing the sole page invalidates every coordinate in it. self.invalidateNodeLayout(chunk.node); const page = chunk.node.page(); erased += page.size.rows; @@ -4652,8 +4698,7 @@ fn eraseRows( continue; } - // Moving the retained suffix changes the meaning and valid range of - // every captured row coordinate in this node. + // Moving the retained suffix changes every captured row coordinate. self.invalidateNodeLayout(chunk.node); // We are modifying our chunk so make sure it is in a good state. @@ -4843,7 +4888,9 @@ pub fn nodeIsValid( target: *List.Node, serial: u64, ) bool { - if (serial < self.page_serial_min) return false; + // Reset invalidates the whole prior epoch, so reject those generations + // without scanning the live list. + if (serial < self.page_serial_epoch) return false; var it = self.pages.first; while (it) |node| : (it = node.next) { @@ -6588,16 +6635,22 @@ fn growColdPagesForTest(self: *PageList, count: usize) !void { } } +/// Fill the current tail page to capacity without allocating a successor. +/// Capturing the tail before the loop makes this stop at the allocation +/// boundary needed by bounded-pruning tests. fn fillLastPageForTest(self: *PageList) !void { const last = self.pages.last.?; while (last.rows() < last.capacity().rows) _ = try self.grow(); } +/// Verify every live page belongs to the current validity epoch, has an +/// allocated generation below the next serial, and validates through the same +/// pointer-plus-generation lookup used by external references. fn expectLivePageSerialsValidForTest(self: *const PageList) !void { const testing = std.testing; var node = self.pages.first; while (node) |live| : (node = live.next) { - try testing.expect(live.serial >= self.page_serial_min); + try testing.expect(live.serial >= self.page_serial_epoch); try testing.expect(live.serial < self.page_serial); try testing.expect(self.nodeIsValid(live, live.serial)); } @@ -7144,6 +7197,7 @@ test "PageList repeated bounded pruning after split preserves live serials" { ); defer s.deinit(); + const epoch = s.page_serial_epoch; while (s.totalPages() < 3) _ = try s.grow(); const first = s.pages.first.?; try s.split(.{ @@ -7153,14 +7207,17 @@ test "PageList repeated bounded pruning after split preserves live serials" { }); // The split target has a fresh serial but precedes older successor pages. - // Prune both the old source and then that target to exercise the serial - // floor after list order has become non-monotonic. + // Prune both the old source and then that target while verifying ordinary + // list mutation does not advance the whole-list validity epoch. for (0..2) |_| { while (s.pages.last.?.rows() < s.pages.last.?.capacity().rows) { _ = try s.grow(); } _ = try s.grow(); + // Ordinary pruning invalidates one generation at a time through live + // list validation; only reset may begin a new whole-list epoch. + try testing.expectEqual(epoch, s.page_serial_epoch); try s.expectLivePageSerialsValidForTest(); } } @@ -16143,7 +16200,7 @@ test "PageList reset invalidates stale untracked refs even if node memory is reu while (stale_len < stale_nodes.len and reused == null) { const old_node = s.pages.first.?; const old_serial = old_node.serial; - try testing.expect(old_serial >= s.page_serial_min); + try testing.expect(old_serial >= s.page_serial_epoch); try testing.expect(old_serial < s.page_serial); stale_nodes[stale_len] = old_node; stale_serials[stale_len] = old_serial; @@ -16169,10 +16226,10 @@ test "PageList reset invalidates stale untracked refs even if node memory is reu // the stale generation before inspecting its pointer, even when that exact // address now belongs to a new live generation. try testing.expectEqual(old_node, new_node); - try testing.expect(old_serial < s.page_serial_min); + try testing.expect(old_serial < s.page_serial_epoch); try testing.expect(!s.nodeIsValid(old_node, old_serial)); try testing.expect(s.nodeIsValid(new_node, new_serial)); - try testing.expect(new_serial >= s.page_serial_min); + try testing.expect(new_serial >= s.page_serial_epoch); try testing.expect(new_serial < s.page_serial); } diff --git a/src/terminal/search/screen.zig b/src/terminal/search/screen.zig index dc683c9e4..859759739 100644 --- a/src/terminal/search/screen.zig +++ b/src/terminal/search/screen.zig @@ -352,10 +352,12 @@ pub const ScreenSearch = struct { const hl = &self.history_results.items[i]; const chunks = hl.chunks.slice(); const serials = chunks.items(.serial); - const lowest = serials[0]; - if (lowest < self.screen.pages.page_serial_min) { - // Everything from here forward we assume is invalid because - // our history results only get older. + const first_serial = serials[0]; + if (first_serial < self.screen.pages.page_serial_epoch) { + // Only a whole-list reset advances the epoch. Results are + // newest to oldest, so this result and the entire remaining + // suffix predate that reset. Drop them without scanning the + // live page list for every captured chunk. const alloc = self.allocator(); if (self.selected) |*m| { const first_pruned = self.active_results.items.len + i; @@ -369,9 +371,9 @@ pub const ScreenSearch = struct { return; } - // A page can also be replaced in place, such as by compaction. - // In that case its old serial remains above page_serial_min, so - // validate the captured pointer and serial against the live list. + // Ordinary pruning, layout changes, and replacements do not + // advance the epoch. Validate their pointer-plus-generation pairs + // against the live list. const nodes = chunks.items(.node); for (nodes, serials) |node, serial| { if (!self.screen.pages.nodeIsValid(node, serial)) break; From 0bad91adb84efc5256f02b4bacf5513695ebac27 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 10 Jul 2026 09:28:14 -0700 Subject: [PATCH 11/12] terminal: renew generations for direct row shifts Screen scroll fast paths and Terminal line insertion/deletion move Row values directly instead of using PageList erasure helpers. Their node generations therefore stayed valid after cached row coordinates changed. Expose the PageList layout invalidator to sibling terminal modules and renew each affected existing page once before full-row rotations or swaps. Keep partial-width cell moves on the content-only path and cover same-page, cross-page, and fresh-tail behavior. --- src/terminal/PageList.zig | 4 ++- src/terminal/Screen.zig | 51 ++++++++++++++++++++++++++++++++++++--- src/terminal/Terminal.zig | 45 ++++++++++++++++++++++++++++++++-- 3 files changed, 93 insertions(+), 7 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 6a5d0d251..79aa96f18 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -3116,6 +3116,8 @@ pub fn scrollClear(self: *PageList) Allocator.Error!void { /// the sole remaining page, or shortening the source page of a split. Without /// a new generation, a cached pointer, serial, and coordinate could still pass /// `nodeIsValid` while referring to a different row than it originally did. +/// Screen and Terminal fast paths which manipulate Page rows directly must use +/// this because they bypass PageList's own row-mutation helpers. /// /// The caller must pass a node which is currently live in this PageList. Call /// this at the point the operation commits to changing the layout and before @@ -3142,7 +3144,7 @@ pub fn scrollClear(self: *PageList) Allocator.Error!void { /// adjust row or memory accounting, mark cells dirty, or notify incremental /// compression. The surrounding operation remains responsible for all such /// bookkeeping required by its layout change. -fn invalidateNodeLayout(self: *PageList, node: *List.Node) void { +pub fn invalidateNodeLayout(self: *PageList, node: *List.Node) void { node.serial = self.page_serial; self.page_serial += 1; } diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 8798411b0..5ca5d957c 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -960,8 +960,8 @@ pub fn cursorScrollAbove(self: *Screen) !void { // lot cheaper in 99% of cases. const old_pin = self.cursor.page_pin.*; - if (try self.pages.grow()) |_| { - try self.cursorScrollAboveRotate(); + if (try self.pages.grow()) |new_node| { + try self.cursorScrollAboveRotate(new_node); } else { // In this case, it means grow() didn't allocate a new page. @@ -981,6 +981,8 @@ pub fn cursorScrollAbove(self: *Screen) !void { // Rotate the rows so that the newly created empty row is at the // beginning. e.g. [ 0 1 2 3 ] in to [ 3 0 1 2 ]. var rows = page.rows.ptr(page.memory.ptr); + // Rotating this suffix changes which logical row its coordinates identify. + self.pages.invalidateNodeLayout(pin.node); fastmem.rotateOnceR(Row, rows[pin.y..page.size.rows]); // Mark the whole page as dirty. @@ -1014,7 +1016,7 @@ pub fn cursorScrollAbove(self: *Screen) !void { // 1 |5E00000000| | 4 // +----------+ : // +-------------+ - try self.cursorScrollAboveRotate(); + try self.cursorScrollAboveRotate(null); } } @@ -1029,7 +1031,10 @@ pub fn cursorScrollAbove(self: *Screen) !void { } } -fn cursorScrollAboveRotate(self: *Screen) !void { +fn cursorScrollAboveRotate( + self: *Screen, + fresh_node: ?*PageList.List.Node, +) !void { self.cursorChangePin(self.cursor.page_pin.down(1).?); // Go through each of the pages following our pin, shift all rows @@ -1042,6 +1047,12 @@ fn cursorScrollAboveRotate(self: *Screen) !void { const prev_rows = prev_page.rows.ptr(prev_page.memory.ptr); const cur_rows = cur_page.rows.ptr(cur_page.memory.ptr); + // A newly allocated tail has no earlier references to invalidate. + if (fresh_node == null or current != fresh_node.?) { + // Rotating this page moves every cached row coordinate down by one. + self.pages.invalidateNodeLayout(current); + } + // Rotate the pages down: [ 0 1 2 3 ] => [ 3 0 1 2 ] fastmem.rotateOnceR(Row, cur_rows[0..cur_page.size.rows]); @@ -1061,6 +1072,8 @@ fn cursorScrollAboveRotate(self: *Screen) !void { assert(current == self.cursor.page_pin.node); const cur_page = current.page(); const cur_rows = cur_page.rows.ptr(cur_page.memory.ptr); + // Rotating the cursor-page suffix changes its cached row coordinates. + self.pages.invalidateNodeLayout(current); fastmem.rotateOnceR(Row, cur_rows[self.cursor.page_pin.y..cur_page.size.rows]); self.clearCells( cur_page, @@ -1145,6 +1158,8 @@ pub fn cursorScrollRegionUp(self: *Screen, limit: usize) !void { // Rotate the region rows so the now-blank top row moves to the // bottom (the cursor row) and everything else shifts up by one. + // Rotating the region changes which logical row its coordinates identify. + self.pages.invalidateNodeLayout(pin.node); fastmem.rotateOnce(Row, rows); // Mark the whole page as dirty. @@ -4919,6 +4934,22 @@ test "Screen: cursorScrollRegionUp simple" { } } +test "Screen: cursorScrollRegionUp renews page generation" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ .cols = 5, .rows = 5, .max_scrollback = 0 }); + defer s.deinit(); + try s.testWriteString("1ABCD\n2EFGH\n3IJKL\n4MNOP\n5QRST"); + s.cursorAbsolute(0, 2); + + const node = s.cursor.page_pin.node; + const serial = node.serial; + try s.cursorScrollRegionUp(2); + + try testing.expect(!s.pages.nodeIsValid(node, serial)); +} + test "Screen: cursorScrollRegionUp moves selection" { const testing = std.testing; const alloc = testing.allocator; @@ -5281,7 +5312,10 @@ test "Screen: scroll above same page" { // +----------+ : // +-------------+ + const node = s.cursor.page_pin.node; + const serial = node.serial; try s.cursorScrollAbove(); + try testing.expect(!s.pages.nodeIsValid(node, serial)); // +----------+ = PAGE 0 // 0 |1ABCD00000| @@ -5356,7 +5390,13 @@ test "Screen: scroll above same page but cursor on previous page" { // +----------+ : // +-------------+ + const first_node = s.pages.pages.first.?; + const second_node = first_node.next.?; + const first_serial = first_node.serial; + const second_serial = second_node.serial; try s.cursorScrollAbove(); + try testing.expect(!s.pages.nodeIsValid(first_node, first_serial)); + try testing.expect(!s.pages.nodeIsValid(second_node, second_serial)); // +----------+ = PAGE 0 // ... : : @@ -5521,7 +5561,10 @@ test "Screen: scroll above creates new page" { // 4307 |3IJKL00000| | 2 // +----------+ : // +-------------+ + const node = s.pages.pages.first.?; + const serial = node.serial; try s.cursorScrollAbove(); + try testing.expect(!s.pages.nodeIsValid(node, serial)); // +----------+ = PAGE 0 // ... : : diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index e5bfd93b7..089201276 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -2429,6 +2429,21 @@ fn rowWillBeShifted( } } +/// Renew every live page generation in an inclusive range before a full-width +/// line operation moves logical rows between their coordinates. +fn invalidateFullWidthRowRange( + self: *Terminal, + first: *PageList.List.Node, + last: *PageList.List.Node, +) void { + var node = first; + while (true) : (node = node.next.?) { + // Full-width line movement remaps cached row coordinates in this page. + self.screens.active.pages.invalidateNodeLayout(node); + if (node == last) break; + } +} + // TODO(qwerasd): `insertLines` and `deleteLines` are 99% identical, // the majority of their logic can (and should) be abstracted in to // a single shared helper function, probably on `Screen` not here. @@ -2504,6 +2519,12 @@ pub fn insertLines(self: *Terminal, count: usize) void { }; defer self.screens.active.pages.untrackPin(cur_p); + // Partial-width margins edit cells in stable rows; full-width moves rows. + if (!left_right) self.invalidateFullWidthRowRange( + self.screens.active.cursor.page_pin.node, + cur_p.node, + ); + // Our current y position relative to the cursor var y: usize = rem; @@ -2699,6 +2720,12 @@ pub fn deleteLines(self: *Terminal, count: usize) void { }; defer self.screens.active.pages.untrackPin(cur_p); + // Partial-width margins edit cells in stable rows; full-width moves rows. + if (!left_right) self.invalidateFullWidthRowRange( + cur_p.node, + cur_p.down(rem - 1).?.node, + ); + // Our current y position relative to the cursor var y: usize = 0; @@ -6867,8 +6894,11 @@ test "Terminal: insertLines simple" { try t.printString("GHI"); t.setCursorPos(2, 2); + const node = t.screens.active.cursor.page_pin.node; + const serial = node.serial; t.clearDirty(); t.insertLines(1); + try testing.expect(!t.screens.active.pages.nodeIsValid(node, serial)); try testing.expect(!t.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); @@ -7046,11 +7076,15 @@ test "Terminal: insertLines across page boundary marks all shifted rows dirty" { try t.printString("5EEEE"); // Verify we now have a second page - try testing.expect(first_page.next != null); + const second_page = first_page.next.?; + const first_serial = first_page.serial; + const second_serial = second_page.serial; t.setCursorPos(1, 1); t.clearDirty(); t.insertLines(1); + try testing.expect(!t.screens.active.pages.nodeIsValid(first_page, first_serial)); + try testing.expect(!t.screens.active.pages.nodeIsValid(second_page, second_serial)); try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); @@ -9754,8 +9788,11 @@ test "Terminal: deleteLines simple" { try t.printString("GHI"); t.setCursorPos(2, 2); + const node = t.screens.active.cursor.page_pin.node; + const serial = node.serial; t.clearDirty(); t.deleteLines(1); + try testing.expect(!t.screens.active.pages.nodeIsValid(node, serial)); try testing.expect(!t.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); @@ -9837,11 +9874,15 @@ test "Terminal: deleteLines across page boundary marks all shifted rows dirty" { try t.printString("5EEEE"); // Verify we now have a second page - try testing.expect(first_page.next != null); + const second_page = first_page.next.?; + const first_serial = first_page.serial; + const second_serial = second_page.serial; t.setCursorPos(1, 1); t.clearDirty(); t.deleteLines(1); + try testing.expect(!t.screens.active.pages.nodeIsValid(first_page, first_serial)); + try testing.expect(!t.screens.active.pages.nodeIsValid(second_page, second_serial)); try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); try testing.expect(t.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); From 89eca063e9f59c1826f5c1017c17417b0d0acfb4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 10 Jul 2026 09:50:40 -0700 Subject: [PATCH 12/12] terminal/search: avoid pruning history on reload reloadActive previously pruned every cached history result whenever the active area changed. Search reconciliation runs under the terminal lock, so this scaled with both result and page counts on a frame-paced path. Validate only the selected history result during reload. Keep full pruning before selection navigation so stale candidates are removed before their coordinates are tracked. --- src/terminal/search/screen.zig | 84 ++++++++++++++++++++++++++++++---- 1 file changed, 75 insertions(+), 9 deletions(-) diff --git a/src/terminal/search/screen.zig b/src/terminal/search/screen.zig index 859759739..e0cd99110 100644 --- a/src/terminal/search/screen.zig +++ b/src/terminal/search/screen.zig @@ -344,6 +344,39 @@ pub const ScreenSearch = struct { } } + fn historyResultIsValid( + self: *const ScreenSearch, + hl: *const FlattenedHighlight, + ) bool { + const chunks = hl.chunks.slice(); + const nodes = chunks.items(.node); + const serials = chunks.items(.serial); + for (nodes, serials) |node, serial| { + if (!self.screen.pages.nodeIsValid(node, serial)) return false; + } + + return true; + } + + /// Clear a selected history match if its flattened page coordinates are + /// stale. reloadActive only inspects the selected history result, so this + /// avoids validating every cached result on each active-area refresh. + fn validateSelectedHistory(self: *ScreenSearch) void { + const m = if (self.selected) |*m| m else return; + const active_len = self.active_results.items.len; + if (m.idx < active_len) return; + + const history_idx = m.idx - active_len; + if (history_idx < self.history_results.items.len and + self.historyResultIsValid(&self.history_results.items[history_idx])) + { + return; + } + + m.deinit(self.screen); + self.selected = null; + } + fn pruneHistory(self: *ScreenSearch) void { // Go through our history results in order (newest to oldest) to find // any result that contains an invalid serial. @@ -374,10 +407,7 @@ pub const ScreenSearch = struct { // Ordinary pruning, layout changes, and replacements do not // advance the epoch. Validate their pointer-plus-generation pairs // against the live list. - const nodes = chunks.items(.node); - for (nodes, serials) |node, serial| { - if (!self.screen.pages.nodeIsValid(node, serial)) break; - } else { + if (self.historyResultIsValid(hl)) { i += 1; continue; } @@ -459,9 +489,9 @@ pub const ScreenSearch = struct { // searchers: column reflow may have freed every node they reference. if (try self.resetIfDimensionsChanged()) return; - // Validate flattened history before any coordinate is converted back - // into a pin or compared with a tracked selection below. - self.pruneHistory(); + // reloadActive only inspects the selected history result, so validate + // that result without scanning every cached match on this hot path. + self.validateSelectedHistory(); const tw = reloadActive_tw; @@ -772,9 +802,11 @@ pub const ScreenSearch = struct { // selected result so no pre-resize pointer is ever dereferenced. _ = try self.resetIfDimensionsChanged(); - // All selection requires valid pins, so reload immediately. Reload - // validates flattened history before inspecting cached coordinates. + // Reload validates the selected history result before inspecting its + // cached coordinates. Prune the remaining results afterward so every + // candidate is valid before selection tracks it. try self.reloadActive(); + self.pruneHistory(); return switch (to) { .next => try self.selectNext(), @@ -1653,6 +1685,40 @@ test "select after partial history page erase ignores shifted results" { try testing.expect(search.selected == null); } +test "reload defers pruning unselected history results" { + const alloc = testing.allocator; + var t: Terminal = try .init(alloc, .{ + .cols = 10, + .rows = 2, + .max_scrollback = std.math.maxInt(usize), + }); + defer t.deinit(alloc); + const list: *PageList = &t.screens.active.pages; + + var stream = t.vtStream(); + defer stream.deinit(); + + const first = list.pages.first.?; + while (first.rows() < first.capacity().rows) stream.nextSlice("\r\n"); + stream.nextSlice("error"); + for (0..list.rows + 1) |_| stream.nextSlice("\r\n"); + + var search: ScreenSearch = try .init(alloc, t.screens.active, "error"); + defer search.deinit(); + try search.searchAll(); + try testing.expectEqual(1, search.history_results.items.len); + + list.eraseHistory(.{ .history = .{ .y = 0 } }); + + // Routine active refreshes don't inspect unselected history results. + try search.reloadActive(); + try testing.expectEqual(1, search.history_results.items.len); + + // Selection prunes every candidate before attempting to track it. + try testing.expect(!try search.select(.next)); + try testing.expectEqual(0, search.history_results.items.len); +} + test "reload after partial history page erase drops shifted selection first" { const alloc = testing.allocator; var t: Terminal = try .init(alloc, .{