diff --git a/src/terminal/search/screen.zig b/src/terminal/search/screen.zig index 179e7da87..c3f48b422 100644 --- a/src/terminal/search/screen.zig +++ b/src/terminal/search/screen.zig @@ -1,6 +1,7 @@ const std = @import("std"); const assert = @import("../../quirks.zig").inlineAssert; const testing = std.testing; +const tripwire = @import("../../tripwire.zig"); const Allocator = std.mem.Allocator; const point = @import("../point.zig"); const highlight = @import("../highlight.zig"); @@ -17,6 +18,11 @@ const SlidingWindow = @import("sliding_window.zig").SlidingWindow; const log = std.log.scoped(.search_screen); +const reloadActive_tw = tripwire.module(enum { + history_append_new, + history_append_existing, +}, ScreenSearch.reloadActive); + /// Searches for a needle within a Screen, handling active area updates, /// pages being pruned from the screen (e.g. scrollback limits), and more. /// @@ -386,6 +392,8 @@ pub const ScreenSearch = struct { /// /// The caller must hold the necessary locks to access the screen state. pub fn reloadActive(self: *ScreenSearch) Allocator.Error!void { + const tw = reloadActive_tw; + // If our selection pin became garbage it means we scrolled off // the end. Clear our selection and on exit of this function, // try to select the last match. @@ -485,12 +493,16 @@ pub const ScreenSearch = struct { alloc, self.history_results.items.len, ); - errdefer results.deinit(alloc); + errdefer { + for (results.items) |*hl| hl.deinit(alloc); + results.deinit(alloc); + } while (window.next()) |hl| { if (hl.chunks.items(.node)[0] == history_node) continue; var hl_cloned = try hl.clone(alloc); errdefer hl_cloned.deinit(alloc); + try tw.check(.history_append_new); try results.append(alloc, hl_cloned); } @@ -505,6 +517,7 @@ pub const ScreenSearch = struct { // Matches! Reverse our list then append all the remaining // history items that didn't start on our original node. std.mem.reverse(FlattenedHighlight, results.items); + try tw.check(.history_append_existing); try results.appendSlice(alloc, self.history_results.items); self.history_results.deinit(alloc); self.history_results = results; @@ -1408,3 +1421,96 @@ test "screen search no scrollback has no history" { defer alloc.free(matches); try testing.expectEqual(0, matches.len); } + +test "reloadActive partial history cleanup on appendSlice error" { + // This test verifies that when reloadActive fails at appendSlice (after + // the loop), all FlattenedHighlight items are properly cleaned up. + 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 s = t.vtStream(); + defer s.deinit(); + + // Write multiple "Fizz" matches that will end up in history. + // We need enough content to push "Fizz" entries into scrollback. + try s.nextSlice("Fizz\r\nFizz\r\n"); + while (list.totalPages() < 3) try s.nextSlice("\r\n"); + for (0..list.rows) |_| try s.nextSlice("\r\n"); + try s.nextSlice("Fizz."); + + // Complete initial search + var search: ScreenSearch = try .init(alloc, t.screens.active, "Fizz"); + defer search.deinit(); + try search.searchAll(); + + // Now trigger reloadActive by adding more content that changes the + // active/history boundary. First add more "Fizz" entries to history. + try s.nextSlice("\r\nFizz\r\nFizz\r\n"); + while (list.totalPages() < 4) try s.nextSlice("\r\n"); + for (0..list.rows) |_| try s.nextSlice("\r\n"); + + // Arm the tripwire to fail at appendSlice (after the loop completes). + // At this point, there are FlattenedHighlight items in the results list + // that need cleanup. + const tw = reloadActive_tw; + defer tw.end(.reset) catch unreachable; + try tw.errorAlways(.history_append_existing, error.OutOfMemory); + + // reloadActive is called by select(), which should trigger the error path. + // If the bug exists, testing.allocator will report a memory leak + // because FlattenedHighlight items weren't cleaned up. + try testing.expectError(error.OutOfMemory, search.select(.next)); +} + +test "reloadActive partial history cleanup on loop append error" { + // This test verifies that when reloadActive fails inside the loop + // (after some items have been appended), all FlattenedHighlight items + // are properly cleaned up. + 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 s = t.vtStream(); + defer s.deinit(); + + // Write multiple "Fizz" matches that will end up in history. + // We need enough content to push "Fizz" entries into scrollback. + try s.nextSlice("Fizz\r\nFizz\r\n"); + while (list.totalPages() < 3) try s.nextSlice("\r\n"); + for (0..list.rows) |_| try s.nextSlice("\r\n"); + try s.nextSlice("Fizz."); + + // Complete initial search + var search: ScreenSearch = try .init(alloc, t.screens.active, "Fizz"); + defer search.deinit(); + try search.searchAll(); + + // Now trigger reloadActive by adding more content that changes the + // active/history boundary. First add more "Fizz" entries to history. + try s.nextSlice("\r\nFizz\r\nFizz\r\n"); + while (list.totalPages() < 4) try s.nextSlice("\r\n"); + for (0..list.rows) |_| try s.nextSlice("\r\n"); + + // Arm the tripwire to fail after the first loop append succeeds. + // This leaves at least one FlattenedHighlight in the results list + // that needs cleanup. + const tw = reloadActive_tw; + defer tw.end(.reset) catch unreachable; + try tw.errorAfter(.history_append_new, error.OutOfMemory, 1); + + // reloadActive is called by select(), which should trigger the error path. + // If the bug exists, testing.allocator will report a memory leak + // because FlattenedHighlight items weren't cleaned up. + try testing.expectError(error.OutOfMemory, search.select(.next)); +}