terminal: fix memory leak on error handling in screen search

This commit is contained in:
Mitchell Hashimoto
2026-01-21 11:56:56 -08:00
parent c1b22a8041
commit 64ccad3a75

View File

@@ -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));
}