mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-07-31 04:39:01 +00:00
terminal: handle page capacity errors in cursorScrollAbove
Re: #13160 When cursorScrollAbove rotates rows across a page boundary, the last row of the previous page is cloned into the destination page. That can cause capacity failures we didn't previously handle. The error propagated out of the operation after rows had already been rotated, leaving the page list half-mutated. Subsequent operations on the corrupted state can cause crashes since the state was incoherent. Handle these errors the same way insertLines and deleteLines already do for their cross-page copies: increase the destination page capacity for the dimension that ran out and retry the row copy.
This commit is contained in:
@@ -1035,36 +1035,107 @@ fn cursorScrollAboveRotate(
|
||||
self: *Screen,
|
||||
fresh_node: ?*PageList.List.Node,
|
||||
) !void {
|
||||
// A fresh node given to us is always the newly allocated tail of
|
||||
// the page list (see grow), which is where our iteration starts.
|
||||
assert(fresh_node == null or fresh_node == self.pages.pages.last);
|
||||
|
||||
self.cursorChangePin(self.cursor.page_pin.down(1).?);
|
||||
|
||||
// Go through each of the pages following our pin, shift all rows
|
||||
// down by one, and copy the last row of the previous page.
|
||||
// down by one, and copy the last row of the previous page. We start
|
||||
// at the tail, which is the only node that can be freshly allocated.
|
||||
var current = self.pages.pages.last.?;
|
||||
while (current != self.cursor.page_pin.node) : (current = current.prev.?) {
|
||||
var current_is_fresh = fresh_node != null;
|
||||
while (current != self.cursor.page_pin.node) : ({
|
||||
current = current.prev.?;
|
||||
current_is_fresh = false;
|
||||
}) {
|
||||
const prev = current.prev.?;
|
||||
const prev_page = prev.page();
|
||||
const cur_page = current.page();
|
||||
const prev_rows = prev_page.rows.ptr(prev_page.memory.ptr);
|
||||
const cur_rows = cur_page.rows.ptr(cur_page.memory.ptr);
|
||||
|
||||
// A newly allocated tail has no earlier references to invalidate.
|
||||
if (fresh_node == null or current != fresh_node.?) {
|
||||
if (!current_is_fresh) {
|
||||
// Rotating this page moves every cached row coordinate down by one.
|
||||
self.pages.invalidateNodeLayout(current);
|
||||
}
|
||||
|
||||
// Rotate the pages down: [ 0 1 2 3 ] => [ 3 0 1 2 ]
|
||||
fastmem.rotateOnceR(Row, cur_rows[0..cur_page.size.rows]);
|
||||
{
|
||||
const cur_page = current.page();
|
||||
const cur_rows = cur_page.rows.ptr(cur_page.memory.ptr);
|
||||
fastmem.rotateOnceR(Row, cur_rows[0..cur_page.size.rows]);
|
||||
}
|
||||
|
||||
// Copy the last row of the previous page to the top of current.
|
||||
try cur_page.cloneRowFrom(
|
||||
prev_page,
|
||||
&cur_rows[0],
|
||||
&prev_rows[prev_page.size.rows - 1],
|
||||
);
|
||||
// If the current page doesn't have enough capacity for the
|
||||
// managed memory of the copied row (styles, hyperlinks, etc.)
|
||||
// then we increase its capacity and retry, the same way that
|
||||
// insertLines/deleteLines handle their cross-page copies.
|
||||
while (true) {
|
||||
const prev_page = prev.page();
|
||||
const cur_page = current.page();
|
||||
const prev_rows = prev_page.rows.ptr(prev_page.memory.ptr);
|
||||
const cur_rows = cur_page.rows.ptr(cur_page.memory.ptr);
|
||||
cur_page.cloneRowFrom(
|
||||
prev_page,
|
||||
&cur_rows[0],
|
||||
&prev_rows[prev_page.size.rows - 1],
|
||||
) catch |err| {
|
||||
// Adjust our page capacity to make
|
||||
// room for what we didn't have space for
|
||||
const new_node = self.increaseCapacity(
|
||||
current,
|
||||
switch (err) {
|
||||
// Rehash the sets
|
||||
error.StyleSetNeedsRehash,
|
||||
error.HyperlinkSetNeedsRehash,
|
||||
=> null,
|
||||
|
||||
// Increase style memory
|
||||
error.StyleSetOutOfMemory,
|
||||
=> .styles,
|
||||
|
||||
// Increase string memory
|
||||
error.StringAllocOutOfMemory,
|
||||
=> .string_bytes,
|
||||
|
||||
// Increase hyperlink memory
|
||||
error.HyperlinkSetOutOfMemory,
|
||||
error.HyperlinkMapOutOfMemory,
|
||||
=> .hyperlink_bytes,
|
||||
|
||||
// Increase grapheme memory
|
||||
error.GraphemeMapOutOfMemory,
|
||||
error.GraphemeAllocOutOfMemory,
|
||||
=> .grapheme_bytes,
|
||||
},
|
||||
) catch |e| switch (e) {
|
||||
// See insertLines which takes the same approach for
|
||||
// the same errors. We can't gracefully recover from
|
||||
// either of these here: the rows have already been
|
||||
// rotated, so returning an error would leave the
|
||||
// page list half-mutated (and corrupt), so a crash
|
||||
// is better.
|
||||
error.OutOfMemory,
|
||||
=> @panic("increaseCapacity system allocator OOM"),
|
||||
|
||||
error.OutOfSpace,
|
||||
=> @panic("increaseCapacity OutOfSpace"),
|
||||
};
|
||||
|
||||
// increaseCapacity replaces the node in the page list
|
||||
// so our iteration node must be updated to the
|
||||
// replacement.
|
||||
current = new_node;
|
||||
|
||||
// Retry the row copy with the increased capacity.
|
||||
continue;
|
||||
};
|
||||
|
||||
break;
|
||||
}
|
||||
|
||||
// Mark dirty on the page, since we are dirtying all rows with this.
|
||||
cur_page.dirty = true;
|
||||
current.page().dirty = true;
|
||||
}
|
||||
|
||||
// Our current is our cursor page, we need to rotate down from
|
||||
@@ -5828,6 +5899,167 @@ test "Screen: scroll above no scrollback bottom of page" {
|
||||
try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } }));
|
||||
}
|
||||
|
||||
test "Screen: scroll above hyperlink-dense row to fresh page" {
|
||||
// Regression test for https://github.com/ghostty-org/ghostty/discussions/13160
|
||||
//
|
||||
// When a scroll-above operation pushes a row carrying more unique
|
||||
// hyperlinks than a fresh page's default hyperlink capacity across
|
||||
// a page boundary, the cross-page row clone must increase the
|
||||
// destination page's capacity (like insertLines/deleteLines do)
|
||||
// rather than error out mid-operation, which leaves the page list
|
||||
// half-mutated and aborts later (e.g. in clearCells).
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
var s = try init(alloc, .{ .cols = 10, .rows = 5, .max_scrollback = 10000 });
|
||||
defer s.deinit();
|
||||
|
||||
// Fill the first page so it is exactly full and the cursor is on
|
||||
// its last row (which is also the bottom row of the active area).
|
||||
// The next grow() will then allocate a fresh page.
|
||||
const first_page_rows = s.pages.pages.first.?.capacity().rows;
|
||||
s.pages.pages.first.?.page().pauseIntegrityChecks(true);
|
||||
for (0..first_page_rows - 1) |_| try s.testWriteString("\n");
|
||||
s.pages.pages.first.?.page().pauseIntegrityChecks(false);
|
||||
try testing.expect(s.pages.pages.first == s.pages.pages.last);
|
||||
try testing.expectEqual(
|
||||
s.pages.pages.first.?.capacity().rows,
|
||||
s.pages.pages.first.?.page().size.rows,
|
||||
);
|
||||
try testing.expectEqual(s.pages.rows - 1, s.cursor.y);
|
||||
|
||||
// Fill the bottom row with unique hyperlinks: more than a fresh
|
||||
// page can hold with default hyperlink capacity.
|
||||
for (0..s.pages.cols) |i| {
|
||||
var buf: [64]u8 = undefined;
|
||||
const uri = try std.fmt.bufPrint(&buf, "http://example.com/{d}", .{i});
|
||||
try s.startHyperlink(uri, null);
|
||||
try s.testWriteString("A");
|
||||
s.endHyperlink();
|
||||
}
|
||||
try testing.expectEqual(
|
||||
@as(usize, s.pages.cols),
|
||||
s.cursor.page_pin.node.page().hyperlink_set.count(),
|
||||
);
|
||||
|
||||
// Move the cursor above the bottom row and scroll. The dense row is
|
||||
// pushed across the page boundary into the freshly allocated page.
|
||||
s.cursorAbsolute(0, 1);
|
||||
try s.cursorScrollAbove();
|
||||
|
||||
// We must have created a second page and the dense row must now be
|
||||
// the top row of that page.
|
||||
try testing.expect(s.pages.pages.first != s.pages.pages.last);
|
||||
|
||||
// All hyperlinks must have survived the scroll intact: every cell
|
||||
// flagged as a hyperlink must resolve to a real entry in its page's
|
||||
// hyperlink map. A half-applied scroll leaves cells whose hyperlink
|
||||
// flag is set but that have no map entry, which aborts in
|
||||
// clearCells later.
|
||||
var node_: ?*PageList.List.Node = s.pages.pages.first;
|
||||
while (node_) |node| : (node_ = node.next) {
|
||||
const page: *Page = node.page();
|
||||
page.assertIntegrity();
|
||||
for (0..page.size.rows) |y| {
|
||||
const row = page.getRow(y);
|
||||
if (!row.hyperlink) continue;
|
||||
for (page.getCells(row)) |*cell| {
|
||||
if (!cell.hyperlink) continue;
|
||||
try testing.expect(page.lookupHyperlink(cell) != null);
|
||||
}
|
||||
}
|
||||
}
|
||||
{
|
||||
const last_page: *Page = s.pages.pages.last.?.page();
|
||||
try testing.expectEqual(
|
||||
@as(usize, s.pages.cols),
|
||||
last_page.hyperlink_set.count(),
|
||||
);
|
||||
}
|
||||
|
||||
// The dense row is still the bottom row of the active area.
|
||||
for (0..s.pages.cols) |x| {
|
||||
const list_cell = s.pages.getCell(.{ .active = .{
|
||||
.x = @intCast(x),
|
||||
.y = 4,
|
||||
} }).?;
|
||||
try testing.expect(list_cell.cell.hyperlink);
|
||||
const page: *Page = list_cell.node.page();
|
||||
const id = page.lookupHyperlink(list_cell.cell).?;
|
||||
const link = page.hyperlink_set.get(page.memory, id);
|
||||
var buf: [64]u8 = undefined;
|
||||
const expected = try std.fmt.bufPrint(&buf, "http://example.com/{d}", .{x});
|
||||
try testing.expectEqualStrings(expected, link.uri.slice(page.memory));
|
||||
}
|
||||
}
|
||||
|
||||
test "Screen: scroll above hyperlink-dense row to existing page" {
|
||||
// Same as the fresh page variant above but the destination page
|
||||
// already exists (fresh_node == null path in cursorScrollAboveRotate).
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
var s = try init(alloc, .{ .cols = 10, .rows = 5, .max_scrollback = 10000 });
|
||||
defer s.deinit();
|
||||
|
||||
// Fill the first page so it is exactly full and the cursor is on
|
||||
// its last row.
|
||||
const first_page_rows = s.pages.pages.first.?.capacity().rows;
|
||||
s.pages.pages.first.?.page().pauseIntegrityChecks(true);
|
||||
for (0..first_page_rows - 1) |_| try s.testWriteString("\n");
|
||||
s.pages.pages.first.?.page().pauseIntegrityChecks(false);
|
||||
try testing.expectEqual(s.pages.rows - 1, s.cursor.y);
|
||||
|
||||
// Fill the last row of the first page with unique hyperlinks:
|
||||
// more than a page can hold with default hyperlink capacity.
|
||||
for (0..s.pages.cols) |i| {
|
||||
var buf: [64]u8 = undefined;
|
||||
const uri = try std.fmt.bufPrint(&buf, "http://example.com/{d}", .{i});
|
||||
try s.startHyperlink(uri, null);
|
||||
try s.testWriteString("A");
|
||||
s.endHyperlink();
|
||||
}
|
||||
|
||||
// Scroll twice so the active area straddles the page boundary:
|
||||
// the last two active rows are on a second page while the dense
|
||||
// row remains the last row of the first page.
|
||||
try s.testWriteString("\n\n");
|
||||
try testing.expect(s.pages.pages.first != s.pages.pages.last);
|
||||
try testing.expect(s.cursor.page_pin.node == s.pages.pages.last.?);
|
||||
|
||||
// Move the cursor to an active row that is still on the first page
|
||||
// and above the dense row, then scroll. grow() has capacity in the
|
||||
// last page so no fresh page is allocated, but the dense row still
|
||||
// crosses the page boundary during the rotate.
|
||||
s.cursorAbsolute(0, 0);
|
||||
try testing.expect(s.cursor.page_pin.node == s.pages.pages.first.?);
|
||||
try s.cursorScrollAbove();
|
||||
|
||||
// All hyperlinks must have survived the scroll intact: every cell
|
||||
// flagged as a hyperlink must resolve to a real entry in its page's
|
||||
// hyperlink map.
|
||||
var node_: ?*PageList.List.Node = s.pages.pages.first;
|
||||
while (node_) |node| : (node_ = node.next) {
|
||||
const page: *Page = node.page();
|
||||
page.assertIntegrity();
|
||||
for (0..page.size.rows) |y| {
|
||||
const row = page.getRow(y);
|
||||
if (!row.hyperlink) continue;
|
||||
for (page.getCells(row)) |*cell| {
|
||||
if (!cell.hyperlink) continue;
|
||||
try testing.expect(page.lookupHyperlink(cell) != null);
|
||||
}
|
||||
}
|
||||
}
|
||||
{
|
||||
const last_page: *Page = s.pages.pages.last.?.page();
|
||||
try testing.expectEqual(
|
||||
@as(usize, s.pages.cols),
|
||||
last_page.hyperlink_set.count(),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
test "Screen: clone" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
Reference in New Issue
Block a user