Misc search fixes (#9711)

- ScreenSearch has to restart on resize. We don't have any good way to
handle reflow since our search results are unpinned. We can look into
this later but this fixes correctness.
- PageList now tracks serial number by node that monotonically increases
on any alloc or reuse. Our screen search uses this to prune invalid
history results.

src/terminal/PageList.zig

@@ -43,6 +43,7 @@ const Node = struct {
     prev: ?*Node = null,
     next: ?*Node = null,
     data: Page,
+    serial: u64,
 };
 
 /// The memory pool we get page nodes from.
@@ -113,6 +114,24 @@ pool_owned: bool,
 /// 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 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.
+///
+/// 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.
+page_serial_min: u64,
+
 /// Byte size of the total amount of allocated pages. Note this does
 /// not include the total allocated amount in the pool which may be more
 /// than this due to preheating.
@@ -264,7 +283,13 @@ pub fn init(
     // necessary.
     var pool = try MemoryPool.init(alloc, std.heap.page_allocator, page_preheat);
     errdefer pool.deinit();
-    const page_list, const page_size = try initPages(&pool, cols, rows);
+    var page_serial: u64 = 0;
+    const page_list, const page_size = try initPages(
+        &pool,
+        &page_serial,
+        cols,
+        rows,
+    );
 
     // Get our minimum max size, see doc comments for more details.
     const min_max_size = try minMaxSize(cols, rows);
@@ -282,6 +307,8 @@ pub fn init(
         .pool = pool,
         .pool_owned = true,
         .pages = page_list,
+        .page_serial = page_serial,
+        .page_serial_min = 0,
         .page_size = page_size,
         .explicit_max_size = max_size orelse std.math.maxInt(usize),
         .min_max_size = min_max_size,
@@ -297,6 +324,7 @@ pub fn init(
 
 fn initPages(
     pool: *MemoryPool,
+    serial: *u64,
     cols: size.CellCountInt,
     rows: size.CellCountInt,
 ) !struct { List, usize } {
@@ -323,6 +351,7 @@ fn initPages(
                 .init(page_buf),
                 Page.layout(cap),
             ),
+            .serial = serial.*,
         };
         node.data.size.rows = @min(rem, node.data.capacity.rows);
         rem -= node.data.size.rows;
@@ -330,6 +359,9 @@ fn initPages(
         // Add the page to the list
         page_list.append(node);
         page_size += page_buf.len;
+
+        // Increment our serial
+        serial.* += 1;
     }
 
     assert(page_list.first != null);
@@ -363,6 +395,7 @@ pub inline fn pauseIntegrityChecks(self: *PageList, pause: bool) void {
 const IntegrityError = error{
     TotalRowsMismatch,
     ViewportPinOffsetMismatch,
+    PageSerialInvalid,
 };
 
 /// Verify the integrity of the PageList. This is expensive and should
@@ -374,8 +407,27 @@ fn verifyIntegrity(self: *const PageList) IntegrityError!void {
     // Our viewport pin should never be garbage
     assert(!self.viewport_pin.garbage);
 
+    // Grab our total rows
+    var actual_total: usize = 0;
+    {
+        var node_ = self.pages.first;
+        while (node_) |node| {
+            actual_total += node.data.size.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) {
+                log.warn(
+                    "PageList integrity violation: page serial too low serial={} min={}",
+                    .{ node.serial, self.page_serial_min },
+                );
+                return IntegrityError.PageSerialInvalid;
+            }
+        }
+    }
+
     // Verify that our cached total_rows matches the actual row count
-    const actual_total = self.totalRows();
     if (actual_total != self.total_rows) {
         log.warn(
             "PageList integrity violation: total_rows mismatch cached={} actual={}",
@@ -523,6 +575,7 @@ pub fn reset(self: *PageList) void {
     // we retained the capacity for the minimum number of pages we need.
     self.pages, self.page_size = initPages(
         &self.pool,
+        &self.page_serial,
         self.cols,
         self.rows,
     ) catch @panic("initPages failed");
@@ -638,6 +691,7 @@ pub fn clone(
     }
 
     // Copy our pages
+    var page_serial: u64 = 0;
     var total_rows: usize = 0;
     var page_size: usize = 0;
     while (it.next()) |chunk| {
@@ -646,6 +700,7 @@ pub fn clone(
         const node = try createPageExt(
             pool,
             chunk.node.data.capacity,
+            &page_serial,
             &page_size,
         );
         assert(node.data.capacity.rows >= chunk.end - chunk.start);
@@ -690,6 +745,8 @@ pub fn clone(
             .alloc => true,
         },
         .pages = page_list,
+        .page_serial = page_serial,
+        .page_serial_min = 0,
         .page_size = page_size,
         .explicit_max_size = self.explicit_max_size,
         .min_max_size = self.min_max_size,
@@ -2431,6 +2488,14 @@ pub fn grow(self: *PageList) !?*List.Node {
         first.data.size.rows = 1;
         self.pages.insertAfter(last, first);
 
+        // 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;
+        first.serial = self.page_serial;
+        self.page_serial += 1;
+
         // Update any tracked pins that point to this page to point to the
         // new first page to the top-left.
         const pin_keys = self.tracked_pins.keys();
@@ -2570,12 +2635,18 @@ inline fn createPage(
     cap: Capacity,
 ) Allocator.Error!*List.Node {
     // log.debug("create page cap={}", .{cap});
-    return try createPageExt(&self.pool, cap, &self.page_size);
+    return try createPageExt(
+        &self.pool,
+        cap,
+        &self.page_serial,
+        &self.page_size,
+    );
 }
 
 inline fn createPageExt(
     pool: *MemoryPool,
     cap: Capacity,
+    serial: *u64,
     total_size: ?*usize,
 ) Allocator.Error!*List.Node {
     var page = try pool.nodes.create();
@@ -2605,8 +2676,12 @@ inline fn createPageExt(
     // to undefined, 0xAA.
     if (comptime std.debug.runtime_safety) @memset(page_buf, 0);
 
-    page.* = .{ .data = .initBuf(.init(page_buf), layout) };
+    page.* = .{
+        .data = .initBuf(.init(page_buf), layout),
+        .serial = serial.*,
+    };
     page.data.size.rows = 0;
+    serial.* += 1;
 
     if (total_size) |v| {
         // Accumulate page size now. We don't assert or check max size

src/terminal/highlight.zig

@@ -114,7 +114,7 @@ pub const Flattened = struct {
     /// The page chunks that make up this highlight. This handles the
     /// y bounds since chunks[0].start is the first highlighted row
     /// and chunks[len - 1].end is the last highlighted row (exclsive).
-    chunks: std.MultiArrayList(PageChunk),
+    chunks: std.MultiArrayList(Chunk),
 
     /// The x bounds of the highlight. `bot_x` may be less than `top_x`
     /// for typical left-to-right highlights: can start the selection right
@@ -122,8 +122,16 @@ pub const Flattened = struct {
     top_x: size.CellCountInt,
     bot_x: size.CellCountInt,
 
-    /// Exposed for easier type references.
-    pub const Chunk = PageChunk;
+    /// A flattened chunk is almost identical to a PageList.Chunk but
+    /// we also flatten the serial number. This lets the flattened
+    /// highlight more robust for comparisons and validity checks with
+    /// the PageList.
+    pub const Chunk = struct {
+        node: *PageList.List.Node,
+        serial: u64,
+        start: size.CellCountInt,
+        end: size.CellCountInt,
+    };
 
     pub const empty: Flattened = .{
         .chunks = .empty,
@@ -139,7 +147,12 @@ pub const Flattened = struct {
         var result: std.MultiArrayList(PageChunk) = .empty;
         errdefer result.deinit(alloc);
         var it = start.pageIterator(.right_down, end);
-        while (it.next()) |chunk| try result.append(alloc, chunk);
+        while (it.next()) |chunk| try result.append(alloc, .{
+            .node = chunk.node,
+            .serial = chunk.node.serial,
+            .start = chunk.start,
+            .end = chunk.end,
+        });
         return .{
             .chunks = result,
             .top_x = start.x,

src/terminal/search/screen.zig

@@ -4,6 +4,7 @@ const testing = std.testing;
 const Allocator = std.mem.Allocator;
 const point = @import("../point.zig");
 const highlight = @import("../highlight.zig");
+const size = @import("../size.zig");
 const FlattenedHighlight = highlight.Flattened;
 const TrackedHighlight = highlight.Tracked;
 const PageList = @import("../PageList.zig");
@@ -57,6 +58,11 @@ pub const ScreenSearch = struct {
     history_results: std.ArrayList(FlattenedHighlight),
     active_results: std.ArrayList(FlattenedHighlight),
 
+    /// The dimensions of the screen. When this changes we need to
+    /// restart the whole search, currently.
+    rows: size.CellCountInt,
+    cols: size.CellCountInt,
+
     pub const SelectedMatch = struct {
         /// Index from the end of the match list (0 = most recent match)
         idx: usize,
@@ -129,6 +135,8 @@ pub const ScreenSearch = struct {
     ) Allocator.Error!ScreenSearch {
         var result: ScreenSearch = .{
             .screen = screen,
+            .rows = screen.pages.rows,
+            .cols = screen.pages.cols,
             .active = try .init(alloc, needle_unowned),
             .history = null,
             .state = .active,
@@ -247,6 +255,29 @@ pub const ScreenSearch = struct {
     /// Feed on a complete screen search will perform some cleanup of
     /// potentially stale history results (pruned) and reclaim some memory.
     pub fn feed(self: *ScreenSearch) Allocator.Error!void {
+        // If the screen resizes, we have to reset our entire search. That
+        // isn't ideal but we don't have a better way right now to handle
+        // reflowing the search results beyond putting a tracked pin for
+        // every single result.
+        if (self.screen.pages.rows != self.rows or
+            self.screen.pages.cols != self.cols)
+        {
+            // Reinit
+            const new: ScreenSearch = try .init(
+                self.allocator(),
+                self.screen,
+                self.needle(),
+            );
+
+            // Deinit/reinit
+            self.deinit();
+            self.* = new;
+
+            // New result should have matching dimensions
+            assert(self.screen.pages.rows == self.rows);
+            assert(self.screen.pages.cols == self.cols);
+        }
+
         const history: *PageListSearch = if (self.history) |*h| &h.searcher else {
             // No history to feed, search is complete.
             self.state = .complete;
@@ -282,49 +313,19 @@ pub const ScreenSearch = struct {
     }
 
     fn pruneHistory(self: *ScreenSearch) void {
-        const history: *PageListSearch = if (self.history) |*h| &h.searcher else return;
-
-        // Keep track of the last checked node to avoid redundant work.
-        var last_checked: ?*PageList.List.Node = null;
-
-        // Go through our history results in reverse order to find
-        // the oldest matches first (since oldest nodes are pruned first).
-        for (0..self.history_results.items.len) |rev_i| {
-            const i = self.history_results.items.len - 1 - rev_i;
-            const node = node: {
+        // 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| {
             const hl = &self.history_results.items[i];
-                break :node hl.chunks.items(.node)[0];
-            };
-
-            // If this is the same node as what we last checked and
-            // found to prune, then continue until we find the first
-            // non-matching, non-pruned node so we can prune the older
-            // ones.
-            if (last_checked == node) continue;
-            last_checked = node;
-
-            // Try to find this node in the PageList using a standard
-            // O(N) traversal. This isn't as bad as it seems because our
-            // oldest matches are likely to be near the start of the
-            // list and as soon as we find one we're done.
-            var it = history.list.pages.first;
-            while (it) |valid_node| : (it = valid_node.next) {
-                if (valid_node != node) continue;
-
-                // This is a valid node. If we're not at rev_i 0 then
-                // it means we have some data to prune! If we are
-                // at rev_i 0 then we can break out because there
-                // is nothing to prune.
-                if (rev_i == 0) return;
-
-                // Prune the last rev_i items.
+            const serials = hl.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 alloc = self.allocator();
-                for (self.history_results.items[i + 1 ..]) |*prune_hl| {
-                    prune_hl.deinit(alloc);
-                }
+                for (self.history_results.items[i..]) |*prune_hl| prune_hl.deinit(alloc);
                 self.history_results.shrinkAndFree(alloc, i);
-
-                // Once we've pruned, future results can't be invalid.
                 return;
             }
         }

src/terminal/search/sliding_window.zig

@@ -87,6 +87,7 @@ pub const SlidingWindow = struct {
     const MetaBuf = CircBuf(Meta, undefined);
     const Meta = struct {
         node: *PageList.List.Node,
+        serial: u64,
         cell_map: std.ArrayList(point.Coordinate),
 
         pub fn deinit(self: *Meta, alloc: Allocator) void {
@@ -345,6 +346,7 @@ pub const SlidingWindow = struct {
                     result.bot_x = end_map.x;
                     self.chunk_buf.appendAssumeCapacity(.{
                         .node = meta.node,
+                        .serial = meta.serial,
                         .start = @intCast(start_map.y),
                         .end = @intCast(end_map.y + 1),
                     });
@@ -363,6 +365,7 @@ pub const SlidingWindow = struct {
                     result.top_x = map.x;
                     self.chunk_buf.appendAssumeCapacity(.{
                         .node = meta.node,
+                        .serial = meta.serial,
                         .start = @intCast(map.y),
                         .end = meta.node.data.size.rows,
                     });
@@ -397,6 +400,7 @@ pub const SlidingWindow = struct {
                     // to our results because we want the full flattened list.
                     self.chunk_buf.appendAssumeCapacity(.{
                         .node = meta.node,
+                        .serial = meta.serial,
                         .start = 0,
                         .end = meta.node.data.size.rows,
                     });
@@ -410,6 +414,7 @@ pub const SlidingWindow = struct {
                 result.bot_x = map.x;
                 self.chunk_buf.appendAssumeCapacity(.{
                     .node = meta.node,
+                    .serial = meta.serial,
                     .start = 0,
                     .end = @intCast(map.y + 1),
                 });
@@ -513,6 +518,7 @@ pub const SlidingWindow = struct {
         // Initialize our metadata for the node.
         var meta: Meta = .{
             .node = node,
+            .serial = node.serial,
             .cell_map = .empty,
         };
         errdefer meta.deinit(self.alloc);