mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-07-10 11:19:45 +00:00
terminal/search: reset cached results after resize
Screen searches only reset cached dimensions while feeding more history. Selecting or reloading a result immediately after a resize left flattened highlights pointing at page nodes freed by reflow. The next selection operation could dereference those stale pointers and crash. Centralize dimension invalidation and run it before feed, reload, and selection paths inspect cached state. Add regression coverage for selecting a cached active match after a column resize.
This commit is contained in:
@@ -182,6 +182,33 @@ pub const ScreenSearch = struct {
|
||||
return self.active_results.items.len + self.history_results.items.len;
|
||||
}
|
||||
|
||||
/// Reinitialize cached search state after a screen resize. Reflow can
|
||||
/// replace every PageList node, so even flattened highlights that weren't
|
||||
/// selected contain invalid node pointers once either dimension changes.
|
||||
///
|
||||
/// Returns true when the reset occurred. The replacement search has
|
||||
/// already loaded its active area through `init` in that case.
|
||||
fn resetIfDimensionsChanged(self: *ScreenSearch) Allocator.Error!bool {
|
||||
if (self.screen.pages.rows == self.rows and
|
||||
self.screen.pages.cols == self.cols)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
const new: ScreenSearch = try .init(
|
||||
self.allocator(),
|
||||
self.screen,
|
||||
self.needle(),
|
||||
);
|
||||
|
||||
self.deinit();
|
||||
self.* = new;
|
||||
|
||||
assert(self.screen.pages.rows == self.rows);
|
||||
assert(self.screen.pages.cols == self.cols);
|
||||
return true;
|
||||
}
|
||||
|
||||
/// Returns all matches as an owned slice (caller must free).
|
||||
/// The matches are ordered from most recent to oldest (e.g. bottom
|
||||
/// of the screen to top of the screen).
|
||||
@@ -260,28 +287,9 @@ 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);
|
||||
}
|
||||
// Resize/reflow invalidates every flattened result, not just search
|
||||
// state that needs another history feed.
|
||||
_ = try self.resetIfDimensionsChanged();
|
||||
|
||||
const history: *PageListSearch = if (self.history) |*h| &h.searcher else {
|
||||
// No history to feed, search is complete.
|
||||
@@ -392,6 +400,10 @@ pub const ScreenSearch = struct {
|
||||
///
|
||||
/// The caller must hold the necessary locks to access the screen state.
|
||||
pub fn reloadActive(self: *ScreenSearch) Allocator.Error!void {
|
||||
// This check must precede all inspection of cached highlights and
|
||||
// searchers: column reflow may have freed every node they reference.
|
||||
if (try self.resetIfDimensionsChanged()) return;
|
||||
|
||||
const tw = reloadActive_tw;
|
||||
|
||||
// If our selection pin became garbage it means we scrolled off
|
||||
@@ -696,6 +708,11 @@ pub const ScreenSearch = struct {
|
||||
/// access to the underlying screen, since we utilize tracked pins to
|
||||
/// ensure our selection sticks with contents changing.
|
||||
pub fn select(self: *ScreenSearch, to: Select) Allocator.Error!bool {
|
||||
// A resize can replace any page node, including nodes retained by
|
||||
// flattened history results. Reset before reloadActive or tracking a
|
||||
// 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.
|
||||
@@ -1090,6 +1107,40 @@ test "select next" {
|
||||
}
|
||||
}
|
||||
|
||||
test "select after resize resets stale flattened results" {
|
||||
const alloc = testing.allocator;
|
||||
var t: Terminal = try .init(alloc, .{
|
||||
.cols = 10,
|
||||
.rows = 3,
|
||||
.max_scrollback = std.math.maxInt(usize),
|
||||
});
|
||||
defer t.deinit(alloc);
|
||||
|
||||
var stream = t.vtStream();
|
||||
defer stream.deinit();
|
||||
stream.nextSlice("Fizz\r\nBuzz");
|
||||
|
||||
var search: ScreenSearch = try .init(alloc, t.screens.active, "Fizz");
|
||||
defer search.deinit();
|
||||
try search.searchAll();
|
||||
try testing.expect(try search.select(.next));
|
||||
try testing.expect(search.selectedMatch() != null);
|
||||
try testing.expectEqual(@as(size.CellCountInt, 10), search.cols);
|
||||
|
||||
// Column reflow replaces the nodes referenced by the cached flattened
|
||||
// active result. select() calls reloadActive(), so dimension invalidation
|
||||
// must run before either path dereferences those old nodes.
|
||||
try t.screens.active.pages.resize(.{ .cols = 5 });
|
||||
try testing.expectEqual(@as(size.CellCountInt, 10), search.cols);
|
||||
|
||||
try testing.expect(try search.select(.next));
|
||||
try testing.expectEqual(@as(size.CellCountInt, 5), search.cols);
|
||||
|
||||
const selected = search.selectedMatch().?.untracked();
|
||||
try testing.expectEqual(@as(size.CellCountInt, 5), selected.start.node.cols());
|
||||
try testing.expectEqual(@as(size.CellCountInt, 5), selected.end.node.cols());
|
||||
}
|
||||
|
||||
test "select in active changes contents completely" {
|
||||
const alloc = testing.allocator;
|
||||
var t: Terminal = try .init(alloc, .{ .cols = 10, .rows = 5 });
|
||||
|
||||
Reference in New Issue
Block a user