mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-07-10 19:29:37 +00:00
terminal: fix increaseCapacity growth from zero-capacity dimensions
PageList.increaseCapacity grows a capacity dimension by doubling it. If the dimension is zero, doubling "succeeds" without growing: the page is reallocated and recloned with an identical capacity, violating the documented guarantee that we always increase by at least one unit. Every unbounded retry site (startHyperlink, cursorSetHyperlink, the reflow probes, insertLines/deleteLines) then loops forever reallocating a page per iteration, and the single-retry sites (styles, graphemes) fail their retry and silently drop data. No production page has a zero dimension today, which is why this has never fired: standard capacities are nonzero and doubling keeps them nonzero. But exactRowCapacity legitimately returns zero for dimensions with no content (a compacted plain text page has zero styles, grapheme, string, and hyperlink capacity), so any compaction work makes this reachable. Growth from zero now jumps straight to the standard default for the dimension rather than doubling. The default is what every standard page starts with, so single-retry callers are guaranteed enough room for their pending allocation, whereas doubling from a minimum unit could still come up short (a single grapheme can need multiple chunks, and a style set below capacity 3 cannot store anything).
This commit is contained in:
@@ -3346,21 +3346,36 @@ pub fn increaseCapacity(
|
||||
const Int = @FieldType(Capacity, field_name);
|
||||
const old = @field(cap, field_name);
|
||||
|
||||
// We use checked math to prevent overflow. If there is an
|
||||
// overflow it means we're out of space in this dimension,
|
||||
// since pages can take up to their maxInt capacity in any
|
||||
// category.
|
||||
const new = std.math.mul(
|
||||
Int,
|
||||
old,
|
||||
2,
|
||||
) catch |err| overflow: {
|
||||
comptime assert(@TypeOf(err) == error{Overflow});
|
||||
// Our final doubling would overflow since maxInt is
|
||||
// 2^N - 1 for an unsignged int of N bits. So, if we overflow
|
||||
// and we haven't used all the bits, use all the bits.
|
||||
if (old < std.math.maxInt(Int)) break :overflow std.math.maxInt(Int);
|
||||
return error.OutOfSpace;
|
||||
const new: Int = new: {
|
||||
// A dimension can be zero for pages with exact
|
||||
// capacities (see compact). Doubling zero stays zero,
|
||||
// which would break our guarantee that we always
|
||||
// increase by at least one unit and turn caller retry
|
||||
// loops into infinite loops. Jump straight to the
|
||||
// standard default instead: it is what all standard
|
||||
// pages start with, so retrying callers are guaranteed
|
||||
// enough room for their pending allocation.
|
||||
if (old == 0) {
|
||||
const default: Capacity = .{ .cols = 0, .rows = 0 };
|
||||
break :new @field(default, field_name);
|
||||
}
|
||||
|
||||
// We use checked math to prevent overflow. If there is
|
||||
// an overflow it means we're out of space in this
|
||||
// dimension, since pages can take up to their maxInt
|
||||
// capacity in any category.
|
||||
break :new std.math.mul(
|
||||
Int,
|
||||
old,
|
||||
2,
|
||||
) catch |err| overflow: {
|
||||
comptime assert(@TypeOf(err) == error{Overflow});
|
||||
// Our final doubling would overflow since maxInt is
|
||||
// 2^N - 1 for an unsignged int of N bits. So, if we overflow
|
||||
// and we haven't used all the bits, use all the bits.
|
||||
if (old < std.math.maxInt(Int)) break :overflow std.math.maxInt(Int);
|
||||
return error.OutOfSpace;
|
||||
};
|
||||
};
|
||||
@field(cap, field_name) = new;
|
||||
|
||||
@@ -14322,6 +14337,40 @@ test "PageList compact oversized page" {
|
||||
}
|
||||
}
|
||||
|
||||
test "PageList increaseCapacity from zero-capacity dimensions" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
var s = try init(alloc, 80, 24, 0);
|
||||
defer s.deinit();
|
||||
|
||||
// Compact the only page. A plain page has no styled, grapheme,
|
||||
// or hyperlink content so the exact capacity is zero in every
|
||||
// managed dimension.
|
||||
var node = (try s.compact(s.pages.first.?)).?;
|
||||
try testing.expectEqual(0, node.data.capacity.styles);
|
||||
try testing.expectEqual(0, node.data.capacity.grapheme_bytes);
|
||||
try testing.expectEqual(0, node.data.capacity.string_bytes);
|
||||
try testing.expectEqual(0, node.data.capacity.hyperlink_bytes);
|
||||
|
||||
// Increasing each dimension from zero must actually grow it.
|
||||
// Regression: 0 * 2 == 0 used to "succeed" without growing,
|
||||
// which turned caller retry loops into infinite loops.
|
||||
node = try s.increaseCapacity(node, .styles);
|
||||
try testing.expect(node.data.capacity.styles > 0);
|
||||
node = try s.increaseCapacity(node, .grapheme_bytes);
|
||||
try testing.expect(node.data.capacity.grapheme_bytes > 0);
|
||||
node = try s.increaseCapacity(node, .string_bytes);
|
||||
try testing.expect(node.data.capacity.string_bytes > 0);
|
||||
node = try s.increaseCapacity(node, .hyperlink_bytes);
|
||||
try testing.expect(node.data.capacity.hyperlink_bytes > 0);
|
||||
|
||||
// Increasing a non-zero dimension still doubles.
|
||||
const styles = node.data.capacity.styles;
|
||||
node = try s.increaseCapacity(node, .styles);
|
||||
try testing.expectEqual(styles * 2, node.data.capacity.styles);
|
||||
}
|
||||
|
||||
test "PageList compact after increaseCapacity" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
@@ -3775,6 +3775,54 @@ test "Screen cursorCopy hyperlink deref" {
|
||||
try testing.expect(s2.cursor.hyperlink_id == 0);
|
||||
}
|
||||
|
||||
test "Screen write regrows compacted page capacity" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
var s = try Screen.init(alloc, .{
|
||||
.cols = 80,
|
||||
.rows = 24,
|
||||
.max_scrollback = 0,
|
||||
});
|
||||
defer s.deinit();
|
||||
|
||||
// Compact the active page so every managed capacity dimension is
|
||||
// zero, then reload the cursor since its cached row/cell pointers
|
||||
// point into the replaced page.
|
||||
{
|
||||
const node = (try s.pages.compact(s.cursor.page_pin.node)).?;
|
||||
try testing.expectEqual(0, node.data.capacity.styles);
|
||||
try testing.expectEqual(0, node.data.capacity.grapheme_bytes);
|
||||
try testing.expectEqual(0, node.data.capacity.string_bytes);
|
||||
try testing.expectEqual(0, node.data.capacity.hyperlink_bytes);
|
||||
s.cursorReload();
|
||||
}
|
||||
|
||||
// Styled write: exercises the manualStyleUpdate single-retry
|
||||
// path. Prior to increaseCapacity handling zero dimensions, the
|
||||
// retry would fail and the style would be dropped.
|
||||
try s.setAttribute(.{ .bold = {} });
|
||||
try s.testWriteString("A");
|
||||
|
||||
// Grapheme write: exercises the appendGrapheme single-retry path.
|
||||
// We can't use testWriteString here because it appends graphemes
|
||||
// directly on the page without the capacity retry.
|
||||
try s.testWriteString("a");
|
||||
try s.appendGrapheme(s.cursorCellLeft(1), 0x0301);
|
||||
|
||||
// Hyperlink: exercises the startHyperlink retry loop, which used
|
||||
// to loop forever when capacity growth from zero didn't grow.
|
||||
try s.startHyperlink("https://example.com/", null);
|
||||
try s.testWriteString("B");
|
||||
s.endHyperlink();
|
||||
|
||||
// Verify the content landed on the page.
|
||||
const page = &s.cursor.page_pin.node.data;
|
||||
try testing.expect(page.styles.count() >= 1);
|
||||
try testing.expect(page.hyperlink_set.count() >= 1);
|
||||
try testing.expect(page.graphemeCount() >= 1);
|
||||
}
|
||||
|
||||
test "Screen cursorCopy hyperlink deref new page" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
Reference in New Issue
Block a user