From 86e1f7b8b53e75809dd8e48d849de502d9397dec Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 23:02:35 -0700 Subject: [PATCH] terminal/search: reject replaced result pages History pruning only compared cached serials with page_serial_min. Replacing a historical node through compaction left its old serial above that cutoff, so selecting the cached match attempted to track a destroyed node and hit the PageList validity assertion. Validate every flattened chunk by finding the live node and matching its serial without dereferencing stale pointers. Remove only the invalid result and adjust any later selection index so unrelated older results remain available. --- src/terminal/PageList.zig | 18 +++++++++ src/terminal/search/screen.zig | 69 ++++++++++++++++++++++++++++++++-- 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index f5203a809..c9c5f3de1 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -4801,6 +4801,24 @@ pub fn pinIsValid(self: *const PageList, p: Pin) bool { return false; } +/// Returns whether a node pointer and serial still identify the same page in +/// this list. The node pointer is only compared and is safe even if the node +/// has been destroyed or reused since the serial was captured. +pub fn nodeIsValid( + self: *const PageList, + target: *List.Node, + serial: u64, +) bool { + if (serial < self.page_serial_min) return false; + + var it = self.pages.first; + while (it) |node| : (it = node.next) { + if (node == target) return node.serial == serial; + } + + return false; +} + /// Returns the viewport for the given pin, preferring to pin to /// "active" if the pin is within the active area. fn pinIsActive(self: *const PageList, p: Pin) bool { diff --git a/src/terminal/search/screen.zig b/src/terminal/search/screen.zig index 2fa2e679e..6b3a487af 100644 --- a/src/terminal/search/screen.zig +++ b/src/terminal/search/screen.zig @@ -327,11 +327,12 @@ pub const ScreenSearch = struct { fn pruneHistory(self: *ScreenSearch) void { // Go through our history results in order (newest to oldest) to find - // any result that contains an invalid serial. Prune up to that - // point. - for (0..self.history_results.items.len) |i| { + // any result that contains an invalid serial. + var i: usize = 0; + while (i < self.history_results.items.len) { const hl = &self.history_results.items[i]; - const serials = hl.chunks.items(.serial); + 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 @@ -348,6 +349,32 @@ pub const ScreenSearch = struct { self.history_results.shrinkAndFree(alloc, i); 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. + const nodes = chunks.items(.node); + for (nodes, serials) |node, serial| { + if (!self.screen.pages.nodeIsValid(node, serial)) break; + } else { + i += 1; + continue; + } + + // Only this result is known to be invalid. Older results may live + // on unrelated pages and remain usable, so remove it individually. + const result_idx = self.active_results.items.len + i; + if (self.selected) |*m| { + if (m.idx == result_idx) { + m.deinit(self.screen); + self.selected = null; + } else if (m.idx > result_idx) { + m.idx -= 1; + } + } + + var removed = self.history_results.orderedRemove(i); + removed.deinit(self.allocator()); } } @@ -1536,6 +1563,40 @@ test "select after partial history erase drops a pruned selection" { try testing.expect(search.selected == null); } +test "select after history compaction ignores replaced 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(); + + stream.nextSlice("error\r\n"); + const first = list.pages.first.?; + while (list.totalPages() < 3) stream.nextSlice("\r\n"); + try list.split(.{ + .node = first, + .y = first.rows() / 2, + .x = 0, + }); + + 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); + + const replacement = (try list.compact(first)).?; + try testing.expect(replacement != first); + + 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, .{