From 95a23f756d0160f814f1043b7b2ebe96f0941482 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 13 Jan 2026 13:09:41 -0800 Subject: [PATCH 01/18] terminal: more strict sizing for page capacities, max cap can fit 64-bit --- src/terminal/PageList.zig | 24 ++++++++++-------------- src/terminal/hyperlink.zig | 5 +++-- src/terminal/page.zig | 34 +++++++++++++++++++++++++++++----- src/terminal/size.zig | 27 +++++++++++++++++++++++++-- src/terminal/style.zig | 2 +- 5 files changed, 68 insertions(+), 24 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index fc680b971..4592b3db6 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -1318,7 +1318,7 @@ const ReflowCursor = struct { // Grow our capacity until we can // definitely fit the extra bytes. const required = cps.len * @sizeOf(u21); - var new_grapheme_capacity: usize = cap.grapheme_bytes; + var new_grapheme_capacity: size.GraphemeBytesInt = cap.grapheme_bytes; while (new_grapheme_capacity - cap.grapheme_bytes < required) { new_grapheme_capacity *= 2; } @@ -1362,7 +1362,7 @@ const ReflowCursor = struct { } else |_| { // Grow our capacity until we can // definitely fit the extra bytes. - var new_string_capacity: usize = cap.string_bytes; + var new_string_capacity: size.StringBytesInt = cap.string_bytes; while (new_string_capacity - cap.string_bytes < additional_required_string_capacity) { new_string_capacity *= 2; } @@ -2647,16 +2647,16 @@ pub const AdjustCapacity = struct { /// Adjust the number of styles in the page. This may be /// rounded up if necessary to fit alignment requirements, /// but it will never be rounded down. - styles: ?usize = null, + styles: ?size.StyleCountInt = null, /// Adjust the number of available grapheme bytes in the page. - grapheme_bytes: ?usize = null, + grapheme_bytes: ?size.GraphemeBytesInt = null, /// Adjust the number of available hyperlink bytes in the page. - hyperlink_bytes: ?usize = null, + hyperlink_bytes: ?size.HyperlinkCountInt = null, /// Adjust the number of available string bytes in the page. - string_bytes: ?usize = null, + string_bytes: ?size.StringBytesInt = null, }; pub const AdjustCapacityError = Allocator.Error || Page.CloneFromError; @@ -2692,23 +2692,19 @@ pub fn adjustCapacity( // All ceilPowerOfTwo is unreachable because we're always same or less // bit width so maxInt is always possible. if (adjustment.styles) |v| { - comptime assert(@bitSizeOf(@TypeOf(v)) <= @bitSizeOf(usize)); - const aligned = std.math.ceilPowerOfTwo(usize, v) catch unreachable; + const aligned = std.math.ceilPowerOfTwo(size.StyleCountInt, v) catch unreachable; cap.styles = @max(cap.styles, aligned); } if (adjustment.grapheme_bytes) |v| { - comptime assert(@bitSizeOf(@TypeOf(v)) <= @bitSizeOf(usize)); - const aligned = std.math.ceilPowerOfTwo(usize, v) catch unreachable; + const aligned = std.math.ceilPowerOfTwo(size.GraphemeBytesInt, v) catch unreachable; cap.grapheme_bytes = @max(cap.grapheme_bytes, aligned); } if (adjustment.hyperlink_bytes) |v| { - comptime assert(@bitSizeOf(@TypeOf(v)) <= @bitSizeOf(usize)); - const aligned = std.math.ceilPowerOfTwo(usize, v) catch unreachable; + const aligned = std.math.ceilPowerOfTwo(size.HyperlinkCountInt, v) catch unreachable; cap.hyperlink_bytes = @max(cap.hyperlink_bytes, aligned); } if (adjustment.string_bytes) |v| { - comptime assert(@bitSizeOf(@TypeOf(v)) <= @bitSizeOf(usize)); - const aligned = std.math.ceilPowerOfTwo(usize, v) catch unreachable; + const aligned = std.math.ceilPowerOfTwo(size.StringBytesInt, v) catch unreachable; cap.string_bytes = @max(cap.string_bytes, aligned); } diff --git a/src/terminal/hyperlink.zig b/src/terminal/hyperlink.zig index 975e6f30e..94f86466c 100644 --- a/src/terminal/hyperlink.zig +++ b/src/terminal/hyperlink.zig @@ -13,8 +13,9 @@ const autoHash = std.hash.autoHash; const autoHashStrat = std.hash.autoHashStrat; /// The unique identifier for a hyperlink. This is at most the number of cells -/// that can fit in a single terminal page. -pub const Id = size.CellCountInt; +/// that can fit in a single terminal page, since each cell can only contain +/// at most one hyperlink. +pub const Id = size.HyperlinkCountInt; // The mapping of cell to hyperlink. We use an offset hash map to save space // since its very unlikely a cell is a hyperlink, so its a waste to store diff --git a/src/terminal/page.zig b/src/terminal/page.zig index 848123405..559f536f1 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -1569,7 +1569,10 @@ pub const Page = struct { const grapheme_alloc_start = alignForward(usize, styles_end, GraphemeAlloc.base_align.toByteUnits()); const grapheme_alloc_end = grapheme_alloc_start + grapheme_alloc_layout.total_size; - const grapheme_count = @divFloor(cap.grapheme_bytes, grapheme_chunk); + const grapheme_count = std.math.ceilPowerOfTwo( + usize, + @divFloor(cap.grapheme_bytes, grapheme_chunk), + ) catch unreachable; const grapheme_map_layout = GraphemeMap.layout(@intCast(grapheme_count)); const grapheme_map_start = alignForward(usize, grapheme_alloc_end, GraphemeMap.base_align.toByteUnits()); const grapheme_map_end = grapheme_map_start + grapheme_map_layout.total_size; @@ -1639,25 +1642,33 @@ pub const Size = struct { }; /// Capacity of this page. +/// +/// This capacity can be maxed out (every field max) and still fit +/// within a 64-bit memory space. If you need more than this, you will +/// need to split data across separate pages. +/// +/// For 32-bit systems, it is possible to overflow the addressable +/// space and this is something we still need to address in the future +/// likely by limiting the maximum capacity on 32-bit systems further. pub const Capacity = struct { /// Number of columns and rows we can know about. cols: size.CellCountInt, rows: size.CellCountInt, /// Number of unique styles that can be used on this page. - styles: usize = 16, + styles: size.StyleCountInt = 16, /// Number of bytes to allocate for hyperlink data. Note that the /// amount of data used for hyperlinks in total is more than this because /// hyperlinks use string data as well as a small amount of lookup metadata. /// This number is a rough approximation. - hyperlink_bytes: usize = hyperlink_bytes_default, + hyperlink_bytes: size.HyperlinkCountInt = hyperlink_bytes_default, /// Number of bytes to allocate for grapheme data. - grapheme_bytes: usize = grapheme_bytes_default, + grapheme_bytes: size.GraphemeBytesInt = grapheme_bytes_default, /// Number of bytes to allocate for strings. - string_bytes: usize = string_bytes_default, + string_bytes: size.StringBytesInt = string_bytes_default, pub const Adjustment = struct { cols: ?size.CellCountInt = null, @@ -2025,6 +2036,19 @@ pub const Cell = packed struct(u64) { // //const pages = total_size / std.heap.page_size_min; // } +test "Page.layout can take a maxed capacity" { + // Our intention is for a maxed-out capacity to always fit + // within a page layout without trigering runtime safety on any + // overflow. This simplifies some of our handling downstream of the + // call (relevant to: https://github.com/ghostty-org/ghostty/issues/10258) + var cap: Capacity = undefined; + inline for (@typeInfo(Capacity).@"struct".fields) |field| { + @field(cap, field.name) = std.math.maxInt(field.type); + } + + _ = Page.layout(cap); +} + test "Cell is zero by default" { const cell = Cell.init(0); const cell_int: u64 = @bitCast(cell); diff --git a/src/terminal/size.zig b/src/terminal/size.zig index 0dedfcc14..7be09739e 100644 --- a/src/terminal/size.zig +++ b/src/terminal/size.zig @@ -11,9 +11,32 @@ pub const max_page_size = std.math.maxInt(u32); /// derived from the maximum terminal page size. pub const OffsetInt = std.math.IntFittingRange(0, max_page_size - 1); -/// The int type that can contain the maximum number of cells in a page. -pub const CellCountInt = u16; // TODO: derive +/// Int types for maximum values of things. A lot of these sizes are +/// based on "X is enough for any reasonable use case" principles. +// The goal is that a user can have the maxInt amount of all of these +// present at one time and be able to address them in a single Page.zig. + +// Total number of cells that are possible in each dimension (row/col). +// Based on 2^16 being enough for any reasonable terminal size and allowing +// IDs to remain 16-bit. +pub const CellCountInt = u16; + +// Total number of styles and hyperlinks that are possible in a page. +// We match CellCountInt here because each cell in a single row can have at +// most one style, making it simple to split a page by splitting rows. // +// Note due to the way RefCountedSet works, we are short one value, but +// this is a theoretical limit we accept. A page with a single row max +// columns wide would be one short of having every cell have a unique style. +pub const StyleCountInt = CellCountInt; +pub const HyperlinkCountInt = CellCountInt; + +// Total number of bytes that can be taken up by grapheme data and string +// data. Both of these technically unlimited with malicious input, but +// we choose a reasonable limit of 2^32 (4GB) per. +pub const GraphemeBytesInt = u32; +pub const StringBytesInt = u32; + /// The offset from the base address of the page to the start of some data. /// This is typed for ease of use. /// diff --git a/src/terminal/style.zig b/src/terminal/style.zig index e5c47b9fe..7908beefa 100644 --- a/src/terminal/style.zig +++ b/src/terminal/style.zig @@ -11,7 +11,7 @@ const RefCountedSet = @import("ref_counted_set.zig").RefCountedSet; /// The unique identifier for a style. This is at most the number of cells /// that can fit into a terminal page. -pub const Id = size.CellCountInt; +pub const Id = size.StyleCountInt; /// The Id to use for default styling. pub const default_id: Id = 0; From 8306f96d9460a440579d002bfcea254fac5889ab Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 13 Jan 2026 13:41:11 -0800 Subject: [PATCH 02/18] terminal: PageList.increaseCapacity --- src/terminal/PageList.zig | 399 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 399 insertions(+) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 4592b3db6..9c63fd578 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -2736,6 +2736,110 @@ pub fn adjustCapacity( return new_node; } +/// Possible dimensions to increase capacity for. +pub const IncreaseCapacity = enum { + styles, + grapheme_bytes, + hyperlink_bytes, + string_bytes, +}; + +pub const IncreaseCapacityError = error{ + // An actual system OOM trying to allocate memory. + OutOfMemory, + + // The existing page is already at max capacity for the given + // adjustment. The caller must create a new page, remove data from + // the old page, etc. (up to the caller). + OutOfSpace, +}; + +/// Increase the capacity of the given page node in the given direction. +/// This will always allocate a new node and remove the old node, so the +/// existing node pointer will be invalid after this call. The newly created +/// node on success is returned. +/// +/// The increase amount is at the control of the PageList implementation, +/// but is guaranteed to always increase by at least one unit in the +/// given dimension. Practically, we'll always increase by much more +/// (we currently double every time) but callers shouldn't depend on that. +/// The only guarantee is some amount of growth. +/// +/// Adjustment can be null if you want to recreate, reclone the page +/// with the same capacity. This is a special case used for rehashing since +/// the logic is otherwise the same. In this case, OutOfMemory is the +/// only possible error. +pub fn increaseCapacity( + self: *PageList, + node: *List.Node, + adjustment: ?IncreaseCapacity, +) IncreaseCapacityError!*List.Node { + defer self.assertIntegrity(); + const page: *Page = &node.data; + + // Apply our adjustment + var cap = page.capacity; + if (adjustment) |v| switch (v) { + inline else => |tag| { + const field_name = @tagName(tag); + 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| { + comptime assert(@TypeOf(err) == error{Overflow}); + return error.OutOfSpace; + }; + @field(cap, field_name) = new; + }, + }; + + log.info("adjusting page capacity={}", .{cap}); + + // Create our new page and clone the old page into it. + const new_node = try self.createPage(cap); + errdefer self.destroyNode(new_node); + const new_page: *Page = &new_node.data; + assert(new_page.capacity.rows >= page.capacity.rows); + assert(new_page.capacity.cols >= page.capacity.cols); + new_page.size.rows = page.size.rows; + new_page.size.cols = page.size.cols; + new_page.cloneFrom( + page, + 0, + page.size.rows, + ) catch |err| { + // cloneFrom only errors if there isn't capacity for the data + // from the source page but we're only increasing capacity so + // this should never be possible. If it happens, we should crash + // because we're in no man's land and can't safely recover. + log.err("increaseCapacity clone failed err={}", .{err}); + @panic("unexpected clone failure"); + }; + + // Must not fail after this because the operations we do after this + // can't be recovered. + errdefer comptime unreachable; + + // Fix up all our tracked pins to point to the new page. + const pin_keys = self.tracked_pins.keys(); + for (pin_keys) |p| { + if (p.node != node) continue; + p.node = new_node; + } + + // Insert this page and destroy the old page + self.pages.insertBefore(node, new_node); + self.pages.remove(node); + self.destroyNode(node); + + new_page.assertIntegrity(); + return new_node; +} + /// Create a new page node. This does not add it to the list and this /// does not do any memory size accounting with max_size/page_size. inline fn createPage( @@ -6482,6 +6586,301 @@ test "PageList adjustCapacity after col shrink" { } } +test "PageList increaseCapacity to increase styles" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 2, 0); + defer s.deinit(); + + const original_styles_cap = s.pages.first.?.data.capacity.styles; + + { + try testing.expect(s.pages.first == s.pages.last); + const page = &s.pages.first.?.data; + + // Write all our data so we can assert its the same after + for (0..s.rows) |y| { + for (0..s.cols) |x| { + const rac = page.getRowAndCell(x, y); + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = @intCast(x) }, + }; + } + } + } + + // Increase our styles + _ = try s.increaseCapacity(s.pages.first.?, .styles); + + { + try testing.expect(s.pages.first == s.pages.last); + const page = &s.pages.first.?.data; + + // Verify capacity doubled + try testing.expectEqual( + original_styles_cap * 2, + page.capacity.styles, + ); + + // Verify data preserved + for (0..s.rows) |y| { + for (0..s.cols) |x| { + const rac = page.getRowAndCell(x, y); + try testing.expectEqual( + @as(u21, @intCast(x)), + rac.cell.content.codepoint, + ); + } + } + } +} + +test "PageList increaseCapacity to increase graphemes" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 2, 0); + defer s.deinit(); + + const original_cap = s.pages.first.?.data.capacity.grapheme_bytes; + + { + try testing.expect(s.pages.first == s.pages.last); + const page = &s.pages.first.?.data; + + for (0..s.rows) |y| { + for (0..s.cols) |x| { + const rac = page.getRowAndCell(x, y); + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = @intCast(x) }, + }; + } + } + } + + _ = try s.increaseCapacity(s.pages.first.?, .grapheme_bytes); + + { + try testing.expect(s.pages.first == s.pages.last); + const page = &s.pages.first.?.data; + + try testing.expectEqual(original_cap * 2, page.capacity.grapheme_bytes); + + for (0..s.rows) |y| { + for (0..s.cols) |x| { + const rac = page.getRowAndCell(x, y); + try testing.expectEqual( + @as(u21, @intCast(x)), + rac.cell.content.codepoint, + ); + } + } + } +} + +test "PageList increaseCapacity to increase hyperlinks" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 2, 0); + defer s.deinit(); + + const original_cap = s.pages.first.?.data.capacity.hyperlink_bytes; + + { + try testing.expect(s.pages.first == s.pages.last); + const page = &s.pages.first.?.data; + + for (0..s.rows) |y| { + for (0..s.cols) |x| { + const rac = page.getRowAndCell(x, y); + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = @intCast(x) }, + }; + } + } + } + + _ = try s.increaseCapacity(s.pages.first.?, .hyperlink_bytes); + + { + try testing.expect(s.pages.first == s.pages.last); + const page = &s.pages.first.?.data; + + try testing.expectEqual(original_cap * 2, page.capacity.hyperlink_bytes); + + for (0..s.rows) |y| { + for (0..s.cols) |x| { + const rac = page.getRowAndCell(x, y); + try testing.expectEqual( + @as(u21, @intCast(x)), + rac.cell.content.codepoint, + ); + } + } + } +} + +test "PageList increaseCapacity to increase string_bytes" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 2, 0); + defer s.deinit(); + + const original_cap = s.pages.first.?.data.capacity.string_bytes; + + { + try testing.expect(s.pages.first == s.pages.last); + const page = &s.pages.first.?.data; + + for (0..s.rows) |y| { + for (0..s.cols) |x| { + const rac = page.getRowAndCell(x, y); + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = @intCast(x) }, + }; + } + } + } + + _ = try s.increaseCapacity(s.pages.first.?, .string_bytes); + + { + try testing.expect(s.pages.first == s.pages.last); + const page = &s.pages.first.?.data; + + try testing.expectEqual(original_cap * 2, page.capacity.string_bytes); + + for (0..s.rows) |y| { + for (0..s.cols) |x| { + const rac = page.getRowAndCell(x, y); + try testing.expectEqual( + @as(u21, @intCast(x)), + rac.cell.content.codepoint, + ); + } + } + } +} + +test "PageList increaseCapacity tracked pins" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 2, 0); + defer s.deinit(); + + // Create a tracked pin on the first page + const tracked = try s.trackPin(s.pin(.{ .active = .{ .x = 1, .y = 1 } }).?); + defer s.untrackPin(tracked); + + const old_node = s.pages.first.?; + try testing.expectEqual(old_node, tracked.node); + + // Increase capacity + const new_node = try s.increaseCapacity(s.pages.first.?, .styles); + + // Pin should now point to the new node + try testing.expectEqual(new_node, tracked.node); + try testing.expectEqual(@as(size.CellCountInt, 1), tracked.x); + try testing.expectEqual(@as(size.CellCountInt, 1), tracked.y); +} + +test "PageList increaseCapacity returns OutOfSpace at max capacity" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 2, 0); + defer s.deinit(); + + // Set styles capacity to a value that will overflow when doubled + // We need to create a page with capacity at more than half of max + const max_styles = std.math.maxInt(size.StyleCountInt); + const half_max = max_styles / 2 + 1; + + // Adjust capacity to near-max first + _ = try s.adjustCapacity(s.pages.first.?, .{ .styles = half_max }); + + // Now increaseCapacity should fail with OutOfSpace + try testing.expectError( + error.OutOfSpace, + s.increaseCapacity(s.pages.first.?, .styles), + ); +} + +test "PageList increaseCapacity after col shrink" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 2, 0); + defer s.deinit(); + + // Shrink columns + try s.resize(.{ .cols = 5, .reflow = false }); + try testing.expectEqual(5, s.cols); + + { + const page = &s.pages.first.?.data; + try testing.expectEqual(5, page.size.cols); + try testing.expect(page.capacity.cols >= 10); + } + + // Increase capacity + _ = try s.increaseCapacity(s.pages.first.?, .styles); + + { + const page = &s.pages.first.?.data; + // size.cols should still be 5, not reverted to capacity.cols + try testing.expectEqual(5, page.size.cols); + try testing.expectEqual(5, s.cols); + } +} + +test "PageList increaseCapacity multi-page" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 80, 24, null); + defer s.deinit(); + + // Grow to create a second page + const page1_node = s.pages.last.?; + page1_node.data.pauseIntegrityChecks(true); + for (0..page1_node.data.capacity.rows - page1_node.data.size.rows) |_| { + try testing.expect(try s.grow() == null); + } + page1_node.data.pauseIntegrityChecks(false); + try testing.expect(try s.grow() != null); + + // Now we have two pages + try testing.expect(s.pages.first != s.pages.last); + const page2_node = s.pages.last.?; + + const page1_styles_cap = s.pages.first.?.data.capacity.styles; + const page2_styles_cap = page2_node.data.capacity.styles; + + // Increase capacity on the first page only + _ = try s.increaseCapacity(s.pages.first.?, .styles); + + // First page capacity should be doubled + try testing.expectEqual( + page1_styles_cap * 2, + s.pages.first.?.data.capacity.styles, + ); + + // Second page should be unchanged + try testing.expectEqual( + page2_styles_cap, + s.pages.last.?.data.capacity.styles, + ); +} + test "PageList pageIterator single page" { const testing = std.testing; const alloc = testing.allocator; From 1e5973386b5eb262526b4990102500e556f6f2ea Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 13 Jan 2026 13:41:11 -0800 Subject: [PATCH 03/18] terminal: Screen.increaseCapacity --- src/terminal/Screen.zig | 312 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 309 insertions(+), 3 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index fed0a8c51..8edb72ba5 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -588,6 +588,76 @@ pub fn adjustCapacity( return new_node; } +pub fn increaseCapacity( + self: *Screen, + node: *PageList.List.Node, + adjustment: ?PageList.IncreaseCapacity, +) PageList.IncreaseCapacityError!*PageList.List.Node { + // If the page being modified isn't our cursor page then + // this is a quick operation because we have no additional + // accounting. We have to do this check here BEFORE calling + // increaseCapacity because increaseCapacity will update all + // our tracked pins (including our cursor). + if (node != self.cursor.page_pin.node) return try self.pages.increaseCapacity( + node, + adjustment, + ); + + // We're modifying the cursor page. When we increase the + // capacity below it will be short the ref count on our + // current style and hyperlink, so we need to init those. + const new_node = try self.pages.increaseCapacity(node, adjustment); + const new_page: *Page = &new_node.data; + + // Re-add the style, if the page somehow doesn't have enough + // memory to add it, we emit a warning and gracefully degrade + // to the default style for the cursor. + if (self.cursor.style_id != style.default_id) { + self.cursor.style_id = new_page.styles.add( + new_page.memory, + self.cursor.style, + ) catch |err| id: { + // TODO: Should we increase the capacity further in this case? + log.warn( + "(Screen.increaseCapacity) Failed to add cursor style back to page, err={}", + .{err}, + ); + + // Reset the cursor style. + self.cursor.style = .{}; + break :id style.default_id; + }; + } + + // Re-add the hyperlink, if the page somehow doesn't have enough + // memory to add it, we emit a warning and gracefully degrade to + // no hyperlink. + if (self.cursor.hyperlink) |link| { + // So we don't attempt to free any memory in the replaced page. + self.cursor.hyperlink_id = 0; + self.cursor.hyperlink = null; + + // Re-add + self.startHyperlinkOnce(link.*) catch |err| { + // TODO: Should we increase the capacity further in this case? + log.warn( + "(Screen.increaseCapacity) Failed to add cursor hyperlink back to page, err={}", + .{err}, + ); + }; + + // Remove our old link + link.deinit(self.alloc); + self.alloc.destroy(link); + } + + // Reload the cursor information because the pin changed. + // So our page row/cell and so on are all off. + self.cursorReload(); + + return new_node; +} + pub inline fn cursorCellRight(self: *Screen, n: size.CellCountInt) *pagepkg.Cell { assert(self.cursor.x + n < self.pages.cols); const cell: [*]pagepkg.Cell = @ptrCast(self.cursor.page_cell); @@ -3007,15 +3077,15 @@ pub fn testWriteString(self: *Screen, text: []const u8) !void { .protected = self.cursor.protected, }; - // If we have a hyperlink, add it to the cell. - if (self.cursor.hyperlink_id > 0) try self.cursorSetHyperlink(); - // If we have a ref-counted style, increase. if (self.cursor.style_id != style.default_id) { const page = self.cursor.page_pin.node.data; page.styles.use(page.memory, self.cursor.style_id); self.cursor.page_row.styled = true; } + + // If we have a hyperlink, add it to the cell. + if (self.cursor.hyperlink_id > 0) try self.cursorSetHyperlink(); }, 2 => { @@ -9103,3 +9173,239 @@ test "Screen: cursorDown to page with insufficient capacity" { try testing.expect(false); } } + +test "Screen: increaseCapacity cursor style ref count preserved" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ + .cols = 5, + .rows = 5, + .max_scrollback = 0, + }); + defer s.deinit(); + try s.setAttribute(.bold); + try s.testWriteString("1ABCD"); + + // We should have one page and it should be our cursor page + try testing.expect(s.pages.pages.first == s.pages.pages.last); + try testing.expect(s.pages.pages.first == s.cursor.page_pin.node); + + const old_style = s.cursor.style; + + { + const page = &s.pages.pages.last.?.data; + // 5 chars + cursor = 6 refs + try testing.expectEqual( + 6, + page.styles.refCount(page.memory, s.cursor.style_id), + ); + } + + // This forces the page to change via increaseCapacity. + const new_node = try s.increaseCapacity( + s.cursor.page_pin.node, + .grapheme_bytes, + ); + + // Cursor's page_pin should now point to the new node + try testing.expect(s.cursor.page_pin.node == new_node); + + // Verify cursor's page_cell and page_row are correctly reloaded from the pin + const page_rac = s.cursor.page_pin.rowAndCell(); + try testing.expect(s.cursor.page_row == page_rac.row); + try testing.expect(s.cursor.page_cell == page_rac.cell); + + // Style should be preserved + try testing.expectEqual(old_style, s.cursor.style); + try testing.expect(s.cursor.style_id != style.default_id); + + // After increaseCapacity, the 5 chars are cloned (5 refs) and + // the cursor's style is re-added (1 ref) = 6 total. + { + const page = &s.pages.pages.last.?.data; + const ref_count = page.styles.refCount(page.memory, s.cursor.style_id); + try testing.expectEqual(6, ref_count); + } +} + +test "Screen: increaseCapacity cursor hyperlink ref count preserved" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ + .cols = 5, + .rows = 5, + .max_scrollback = 0, + }); + defer s.deinit(); + try s.startHyperlink("https://example.com/", null); + try s.testWriteString("1ABCD"); + + // We should have one page and it should be our cursor page + try testing.expect(s.pages.pages.first == s.pages.pages.last); + try testing.expect(s.pages.pages.first == s.cursor.page_pin.node); + + { + const page = &s.pages.pages.last.?.data; + // Cursor has the hyperlink active = 1 count in hyperlink_set + try testing.expectEqual(1, page.hyperlink_set.count()); + try testing.expect(s.cursor.hyperlink_id != 0); + try testing.expect(s.cursor.hyperlink != null); + } + + // This forces the page to change via increaseCapacity. + _ = try s.increaseCapacity( + s.cursor.page_pin.node, + .grapheme_bytes, + ); + + // Hyperlink should be preserved with correct URI + try testing.expect(s.cursor.hyperlink != null); + try testing.expect(s.cursor.hyperlink_id != 0); + try testing.expectEqualStrings("https://example.com/", s.cursor.hyperlink.?.uri); + + // After increaseCapacity, the hyperlink is re-added to the new page. + { + const page = &s.pages.pages.last.?.data; + try testing.expectEqual(1, page.hyperlink_set.count()); + } +} + +test "Screen: increaseCapacity cursor with both style and hyperlink preserved" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ + .cols = 5, + .rows = 5, + .max_scrollback = 0, + }); + defer s.deinit(); + + // Set both a non-default style AND an active hyperlink. + // Write one character first with bold to mark the row as styled, + // then start the hyperlink and write more characters. + try s.setAttribute(.bold); + try s.startHyperlink("https://example.com/", null); + try s.testWriteString("1ABCD"); + + // We should have one page and it should be our cursor page + try testing.expect(s.pages.pages.first == s.pages.pages.last); + try testing.expect(s.pages.pages.first == s.cursor.page_pin.node); + + const old_style = s.cursor.style; + + { + const page = &s.pages.pages.last.?.data; + // 5 chars + cursor = 6 refs for bold style + try testing.expectEqual( + 6, + page.styles.refCount(page.memory, s.cursor.style_id), + ); + // Cursor has the hyperlink active = 1 count in hyperlink_set + try testing.expectEqual(1, page.hyperlink_set.count()); + try testing.expect(s.cursor.style_id != style.default_id); + try testing.expect(s.cursor.hyperlink_id != 0); + try testing.expect(s.cursor.hyperlink != null); + } + + // This forces the page to change via increaseCapacity. + _ = try s.increaseCapacity( + s.cursor.page_pin.node, + .grapheme_bytes, + ); + + // Style should be preserved + try testing.expectEqual(old_style, s.cursor.style); + try testing.expect(s.cursor.style_id != style.default_id); + + // Hyperlink should be preserved with correct URI + try testing.expect(s.cursor.hyperlink != null); + try testing.expect(s.cursor.hyperlink_id != 0); + try testing.expectEqualStrings("https://example.com/", s.cursor.hyperlink.?.uri); + + // After increaseCapacity, both style and hyperlink are re-added to the new page. + { + const page = &s.pages.pages.last.?.data; + const ref_count = page.styles.refCount(page.memory, s.cursor.style_id); + try testing.expectEqual(6, ref_count); + try testing.expectEqual(1, page.hyperlink_set.count()); + } +} + +test "Screen: increaseCapacity non-cursor page returns early" { + // Test that calling increaseCapacity on a page that is NOT the cursor's + // page properly delegates to pages.increaseCapacity without doing the + // extra cursor accounting (style/hyperlink re-adding). + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ + .cols = 80, + .rows = 24, + .max_scrollback = 10000, + }); + defer s.deinit(); + + // Set up a custom style and hyperlink on the cursor + try s.setAttribute(.bold); + try s.startHyperlink("https://example.com/", null); + try s.testWriteString("Hello"); + + // Store cursor state before growing pages + const old_style = s.cursor.style; + const old_style_id = s.cursor.style_id; + const old_hyperlink = s.cursor.hyperlink; + const old_hyperlink_id = s.cursor.hyperlink_id; + + // The cursor is on the first (and only) page + try testing.expect(s.pages.pages.first == s.pages.pages.last); + try testing.expect(s.cursor.page_pin.node == s.pages.pages.first.?); + + // Grow pages until we have multiple pages. The cursor's pin stays on + // the first page since we're just adding rows. + const first_page_node = s.pages.pages.first.?; + first_page_node.data.pauseIntegrityChecks(true); + for (0..first_page_node.data.capacity.rows - first_page_node.data.size.rows) |_| { + _ = try s.pages.grow(); + } + first_page_node.data.pauseIntegrityChecks(false); + _ = try s.pages.grow(); + + // Now we have two pages + try testing.expect(s.pages.pages.first != s.pages.pages.last); + const second_page = s.pages.pages.last.?; + + // Cursor should still be on the first page (where it was created) + try testing.expect(s.cursor.page_pin.node == s.pages.pages.first.?); + try testing.expect(s.cursor.page_pin.node != second_page); + + const second_page_styles_cap = second_page.data.capacity.styles; + const cursor_page_styles_cap = s.cursor.page_pin.node.data.capacity.styles; + + // Call increaseCapacity on the second page (NOT the cursor's page) + const new_second_page = try s.increaseCapacity(second_page, .styles); + + // The second page should have increased capacity + try testing.expectEqual( + second_page_styles_cap * 2, + new_second_page.data.capacity.styles, + ); + + // The cursor's page (first page) should be unchanged + try testing.expectEqual( + cursor_page_styles_cap, + s.cursor.page_pin.node.data.capacity.styles, + ); + + // Cursor state should be completely unchanged since we didn't touch its page + try testing.expectEqual(old_style, s.cursor.style); + try testing.expectEqual(old_style_id, s.cursor.style_id); + try testing.expectEqual(old_hyperlink, s.cursor.hyperlink); + try testing.expectEqual(old_hyperlink_id, s.cursor.hyperlink_id); + + // Verify hyperlink is still valid + try testing.expect(s.cursor.hyperlink != null); + try testing.expectEqualStrings("https://example.com/", s.cursor.hyperlink.?.uri); +} From 29d4aba03337d7e9d6a0c357969e8924240b84dd Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Jan 2026 09:07:44 -0800 Subject: [PATCH 04/18] terminal: Screen replace adjust with increaseCapacity --- src/terminal/Screen.zig | 101 ++++++++++++++++++++++++++++------------ src/terminal/page.zig | 2 +- 2 files changed, 72 insertions(+), 31 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 8edb72ba5..66dc37006 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -1176,14 +1176,19 @@ inline fn cursorChangePin(self: *Screen, new: Pin) void { return; } - // If we have a old style then we need to release it from the old page. + // If we have an old style then we need to release it from the old page. const old_style_: ?style.Style = if (self.cursor.style_id == style.default_id) null else self.cursor.style; if (old_style_ != null) { + // Release the style directly from the old page instead of going through + // manualStyleUpdate, because the cursor position may have already been + // updated but the pin has not, which would fail integrity checks. + const old_page: *Page = &self.cursor.page_pin.node.data; + old_page.styles.release(old_page.memory, self.cursor.style_id); self.cursor.style = .{}; - self.manualStyleUpdate() catch unreachable; // Removing a style should never fail + self.cursor.style_id = style.default_id; } // If we have a hyperlink then we need to release it from the old page. @@ -2000,7 +2005,17 @@ pub fn setAttribute(self: *Screen, attr: sgr.Attribute) !void { } /// Call this whenever you manually change the cursor style. -pub fn manualStyleUpdate(self: *Screen) !void { +/// +/// Note that this can return any PageList capacity error, because it +/// is possible for the internal pagelist to not accommodate the new style +/// at all. This WILL attempt to resize our internal pages to fit the style +/// but it is possible that it cannot be done, in which case upstream callers +/// need to split the page or do something else. +/// +/// NOTE(mitchellh): I think in the future we'll do page splitting +/// automatically here and remove this failure scenario. +pub fn manualStyleUpdate(self: *Screen) PageList.IncreaseCapacityError!void { + defer self.assertIntegrity(); var page: *Page = &self.cursor.page_pin.node.data; // std.log.warn("active styles={}", .{page.styles.count()}); @@ -2019,6 +2034,9 @@ pub fn manualStyleUpdate(self: *Screen) !void { // Clear the cursor style ID to prevent weird things from happening // if the page capacity has to be adjusted which would end up calling // manualStyleUpdate again. + // + // This also ensures that if anything fails below, we fall back to + // clearing our style. self.cursor.style_id = style.default_id; // After setting the style, we need to update our style map. @@ -2030,30 +2048,50 @@ pub fn manualStyleUpdate(self: *Screen) !void { page.memory, self.cursor.style, ) catch |err| id: { - // Our style map is full or needs to be rehashed, - // so we allocate a new page, which will rehash, - // and double the style capacity for it if it was - // full. - const node = try self.adjustCapacity( + // Our style map is full or needs to be rehashed, so we need to + // increase style capacity (or rehash). + const node = try self.increaseCapacity( self.cursor.page_pin.node, switch (err) { - error.OutOfMemory => .{ .styles = page.capacity.styles * 2 }, - error.NeedsRehash => .{}, + error.OutOfMemory => .styles, + error.NeedsRehash => null, }, ); page = &node.data; - break :id try page.styles.add( + break :id page.styles.add( page.memory, self.cursor.style, - ); + ) catch |err2| switch (err2) { + error.OutOfMemory => { + // This shouldn't happen because increaseCapacity is + // guaranteed to increase our capacity by at least one and + // we only need one space, but again, I don't want to crash + // here so let's log loudly and reset. + log.err("style addition failed after capacity increase", .{}); + return error.OutOfMemory; + }, + error.NeedsRehash => { + // This should be impossible because we rehash above + // and rehashing should never result in a duplicate. But + // we don't want to simply hard crash so log it and + // clear our style. + log.err("style rehash resulted in needs rehash", .{}); + return; + }, + }; }; + errdefer page.styles.release(page.memory, id); + self.cursor.style_id = id; - self.assertIntegrity(); } /// Append a grapheme to the given cell within the current cursor row. -pub fn appendGrapheme(self: *Screen, cell: *Cell, cp: u21) !void { +pub fn appendGrapheme( + self: *Screen, + cell: *Cell, + cp: u21, +) PageList.IncreaseCapacityError!void { defer self.cursor.page_pin.node.data.assertIntegrity(); self.cursor.page_pin.node.data.appendGrapheme( self.cursor.page_row, @@ -2073,11 +2111,9 @@ pub fn appendGrapheme(self: *Screen, cell: *Cell, cp: u21) !void { // Adjust our capacity. This will update our cursor page pin and // force us to reload. - const original_node = self.cursor.page_pin.node; - const new_bytes = original_node.data.capacity.grapheme_bytes * 2; - _ = try self.adjustCapacity( - original_node, - .{ .grapheme_bytes = new_bytes }, + _ = try self.increaseCapacity( + self.cursor.page_pin.node, + .grapheme_bytes, ); // The cell pointer is now invalid, so we need to get it from @@ -2088,17 +2124,22 @@ pub fn appendGrapheme(self: *Screen, cell: *Cell, cp: u21) !void { .gt => self.cursorCellRight(@intCast(cell_idx - self.cursor.x)), }; - try self.cursor.page_pin.node.data.appendGrapheme( + self.cursor.page_pin.node.data.appendGrapheme( self.cursor.page_row, reloaded_cell, cp, - ); + ) catch |err2| { + comptime assert(@TypeOf(err2) == error{OutOfMemory}); + // This should never happen because we just increased capacity. + // Log loudly but still return an error so we don't just + // crash. + log.err("grapheme append failed after capacity increase", .{}); + return err2; + }; }, }; } -pub const StartHyperlinkError = Allocator.Error || PageList.AdjustCapacityError; - /// Start the hyperlink state. Future cells will be marked as hyperlinks with /// this state. Note that various terminal operations may clear the hyperlink /// state, such as switching screens (alt screen). @@ -2106,7 +2147,7 @@ pub fn startHyperlink( self: *Screen, uri: []const u8, id_: ?[]const u8, -) StartHyperlinkError!void { +) PageList.IncreaseCapacityError!void { // Create our pending entry. const link: hyperlink.Hyperlink = .{ .uri = uri, @@ -2131,21 +2172,21 @@ pub fn startHyperlink( error.OutOfMemory => return error.OutOfMemory, // strings table is out of memory, adjust it up - error.StringsOutOfMemory => _ = try self.adjustCapacity( + error.StringsOutOfMemory => _ = try self.increaseCapacity( self.cursor.page_pin.node, - .{ .string_bytes = self.cursor.page_pin.node.data.capacity.string_bytes * 2 }, + .string_bytes, ), // hyperlink set is out of memory, adjust it up - error.SetOutOfMemory => _ = try self.adjustCapacity( + error.SetOutOfMemory => _ = try self.increaseCapacity( self.cursor.page_pin.node, - .{ .hyperlink_bytes = self.cursor.page_pin.node.data.capacity.hyperlink_bytes * 2 }, + .hyperlink_bytes, ), // hyperlink set is too full, rehash it - error.SetNeedsRehash => _ = try self.adjustCapacity( + error.SetNeedsRehash => _ = try self.increaseCapacity( self.cursor.page_pin.node, - .{}, + null, ), } diff --git a/src/terminal/page.zig b/src/terminal/page.zig index 559f536f1..1150027a4 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -2038,7 +2038,7 @@ pub const Cell = packed struct(u64) { test "Page.layout can take a maxed capacity" { // Our intention is for a maxed-out capacity to always fit - // within a page layout without trigering runtime safety on any + // within a page layout without triggering runtime safety on any // overflow. This simplifies some of our handling downstream of the // call (relevant to: https://github.com/ghostty-org/ghostty/issues/10258) var cap: Capacity = undefined; From 25b7cc9f2cc28071d9d07f3a96ab86c811f1d1e1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Jan 2026 10:56:32 -0800 Subject: [PATCH 05/18] terminal: hyperlink state uses increaseCapacity on screen --- src/terminal/Screen.zig | 48 ++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 66dc37006..f524bd7f4 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2248,7 +2248,7 @@ pub fn endHyperlink(self: *Screen) void { } /// Set the current hyperlink state on the current cell. -pub fn cursorSetHyperlink(self: *Screen) !void { +pub fn cursorSetHyperlink(self: *Screen) PageList.IncreaseCapacityError!void { assert(self.cursor.hyperlink_id != 0); var page = &self.cursor.page_pin.node.data; @@ -2263,10 +2263,6 @@ pub fn cursorSetHyperlink(self: *Screen) !void { } else |err| switch (err) { // hyperlink_map is out of space, realloc the page to be larger error.HyperlinkMapOutOfMemory => { - const uri_size = if (self.cursor.hyperlink) |link| link.uri.len else 0; - - var string_bytes = page.capacity.string_bytes; - // Attempt to allocate the space that would be required to // insert a new copy of the cursor hyperlink uri in to the // string alloc, since right now adjustCapacity always just @@ -2274,29 +2270,31 @@ pub fn cursorSetHyperlink(self: *Screen) !void { // If this alloc fails then we know we also need to grow our // string bytes. // - // FIXME: This SUCKS - if (page.string_alloc.alloc( - u8, - page.memory, - uri_size, - )) |slice| { - // We don't bother freeing because we're - // about to free the entire page anyway. - _ = &slice; - } else |_| { - // We didn't have enough room, let's just double our - // string bytes until there's definitely enough room - // for our uri. - const before = string_bytes; - while (string_bytes - before < uri_size) string_bytes *= 2; + // FIXME: increaseCapacity should not do this. + while (self.cursor.hyperlink) |link| { + if (page.string_alloc.alloc( + u8, + page.memory, + link.uri.len, + )) |slice| { + // We don't bother freeing because we're + // about to free the entire page anyway. + _ = slice; + break; + } else |_| {} + + // We didn't have enough room, let's increase string bytes + const new_node = try self.increaseCapacity( + self.cursor.page_pin.node, + .string_bytes, + ); + assert(new_node == self.cursor.page_pin.node); + page = &new_node.data; } - _ = try self.adjustCapacity( + _ = try self.increaseCapacity( self.cursor.page_pin.node, - .{ - .hyperlink_bytes = page.capacity.hyperlink_bytes * 2, - .string_bytes = string_bytes, - }, + .hyperlink_bytes, ); // Retry From c8afc423082ef764c9f5e62b5269cc5dc2fcff22 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Jan 2026 11:37:58 -0800 Subject: [PATCH 06/18] terminal: switch to increaseCapacity --- src/terminal/Terminal.zig | 72 ++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 43 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index d717a9724..310adaf29 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -1634,54 +1634,48 @@ pub fn insertLines(self: *Terminal, count: usize) void { self.scrolling_region.left, self.scrolling_region.right + 1, ) catch |err| { - const cap = dst_p.node.data.capacity; // Adjust our page capacity to make // room for we didn't have space for - _ = self.screens.active.adjustCapacity( + _ = self.screens.active.increaseCapacity( dst_p.node, switch (err) { // Rehash the sets error.StyleSetNeedsRehash, error.HyperlinkSetNeedsRehash, - => .{}, + => null, // Increase style memory error.StyleSetOutOfMemory, - => .{ .styles = cap.styles * 2 }, + => .styles, // Increase string memory error.StringAllocOutOfMemory, - => .{ .string_bytes = cap.string_bytes * 2 }, + => .string_bytes, // Increase hyperlink memory error.HyperlinkSetOutOfMemory, error.HyperlinkMapOutOfMemory, - => .{ .hyperlink_bytes = cap.hyperlink_bytes * 2 }, + => .hyperlink_bytes, // Increase grapheme memory error.GraphemeMapOutOfMemory, error.GraphemeAllocOutOfMemory, - => .{ .grapheme_bytes = cap.grapheme_bytes * 2 }, + => .grapheme_bytes, }, ) catch |e| switch (e) { - // This shouldn't be possible because above we're only - // adjusting capacity _upwards_. So it should have all - // the existing capacity it had to fit the adjusted - // data. Panic since we don't expect this. - error.StyleSetOutOfMemory, - error.StyleSetNeedsRehash, - error.StringAllocOutOfMemory, - error.HyperlinkSetOutOfMemory, - error.HyperlinkSetNeedsRehash, - error.HyperlinkMapOutOfMemory, - error.GraphemeMapOutOfMemory, - error.GraphemeAllocOutOfMemory, - => @panic("adjustCapacity resulted in capacity errors"), - - // The system allocator is OOM. We can't currently do - // anything graceful here. We panic. + // System OOM. We have no way to recover from this + // currently. We should probably change insertLines + // to raise an error here. error.OutOfMemory, - => @panic("adjustCapacity system allocator OOM"), + => @panic("increaseCapacity system allocator OOM"), + + // The page can't accomodate the managed memory required + // for this operation. We previously just corrupted + // memory here so a crash is better. The right long + // term solution is to allocate a new page here + // move this row to the new page, and start over. + error.OutOfSpace, + => @panic("increaseCapacity OutOfSpace"), }; // Continue the loop to try handling this row again. @@ -1834,49 +1828,41 @@ pub fn deleteLines(self: *Terminal, count: usize) void { self.scrolling_region.left, self.scrolling_region.right + 1, ) catch |err| { - const cap = dst_p.node.data.capacity; // Adjust our page capacity to make // room for we didn't have space for - _ = self.screens.active.adjustCapacity( + _ = self.screens.active.increaseCapacity( dst_p.node, switch (err) { // Rehash the sets error.StyleSetNeedsRehash, error.HyperlinkSetNeedsRehash, - => .{}, + => null, // Increase style memory error.StyleSetOutOfMemory, - => .{ .styles = cap.styles * 2 }, + => .styles, // Increase string memory error.StringAllocOutOfMemory, - => .{ .string_bytes = cap.string_bytes * 2 }, + => .string_bytes, // Increase hyperlink memory error.HyperlinkSetOutOfMemory, error.HyperlinkMapOutOfMemory, - => .{ .hyperlink_bytes = cap.hyperlink_bytes * 2 }, + => .hyperlink_bytes, // Increase grapheme memory error.GraphemeMapOutOfMemory, error.GraphemeAllocOutOfMemory, - => .{ .grapheme_bytes = cap.grapheme_bytes * 2 }, + => .grapheme_bytes, }, ) catch |e| switch (e) { - // See insertLines which has the same error capture. - error.StyleSetOutOfMemory, - error.StyleSetNeedsRehash, - error.StringAllocOutOfMemory, - error.HyperlinkSetOutOfMemory, - error.HyperlinkSetNeedsRehash, - error.HyperlinkMapOutOfMemory, - error.GraphemeMapOutOfMemory, - error.GraphemeAllocOutOfMemory, - => @panic("adjustCapacity resulted in capacity errors"), - + // See insertLines error.OutOfMemory, - => @panic("adjustCapacity system allocator OOM"), + => @panic("increaseCapacity system allocator OOM"), + + error.OutOfSpace, + => @panic("increaseCapacity OutOfSpace"), }; // Continue the loop to try handling this row again. From b59ac60a8772ae1e259966efc0eb9feb5c4167cb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Jan 2026 11:40:39 -0800 Subject: [PATCH 07/18] terminal: remove Screen.adjustCapacity --- src/terminal/Screen.zig | 359 +++++++++----------------------------- src/terminal/Terminal.zig | 2 +- 2 files changed, 80 insertions(+), 281 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index f524bd7f4..1538da5da 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -517,77 +517,6 @@ pub fn clone( result.assertIntegrity(); return result; } - -/// Adjust the capacity of a page within the pagelist of this screen. -/// This handles some accounting if the page being modified is the -/// cursor page. -pub fn adjustCapacity( - self: *Screen, - node: *PageList.List.Node, - adjustment: PageList.AdjustCapacity, -) PageList.AdjustCapacityError!*PageList.List.Node { - // If the page being modified isn't our cursor page then - // this is a quick operation because we have no additional - // accounting. - if (node != self.cursor.page_pin.node) { - return try self.pages.adjustCapacity(node, adjustment); - } - - // We're modifying the cursor page. When we adjust the - // capacity below it will be short the ref count on our - // current style and hyperlink, so we need to init those. - const new_node = try self.pages.adjustCapacity(node, adjustment); - const new_page: *Page = &new_node.data; - - // Re-add the style, if the page somehow doesn't have enough - // memory to add it, we emit a warning and gracefully degrade - // to the default style for the cursor. - if (self.cursor.style_id != 0) { - self.cursor.style_id = new_page.styles.add( - new_page.memory, - self.cursor.style, - ) catch |err| id: { - // TODO: Should we increase the capacity further in this case? - log.warn( - "(Screen.adjustCapacity) Failed to add cursor style back to page, err={}", - .{err}, - ); - - // Reset the cursor style. - self.cursor.style = .{}; - break :id style.default_id; - }; - } - - // Re-add the hyperlink, if the page somehow doesn't have enough - // memory to add it, we emit a warning and gracefully degrade to - // no hyperlink. - if (self.cursor.hyperlink) |link| { - // So we don't attempt to free any memory in the replaced page. - self.cursor.hyperlink_id = 0; - self.cursor.hyperlink = null; - - // Re-add - self.startHyperlinkOnce(link.*) catch |err| { - // TODO: Should we increase the capacity further in this case? - log.warn( - "(Screen.adjustCapacity) Failed to add cursor hyperlink back to page, err={}", - .{err}, - ); - }; - - // Remove our old link - link.deinit(self.alloc); - self.alloc.destroy(link); - } - - // Reload the cursor information because the pin changed. - // So our page row/cell and so on are all off. - self.cursorReload(); - - return new_node; -} - pub fn increaseCapacity( self: *Screen, node: *PageList.List.Node, @@ -2265,7 +2194,7 @@ pub fn cursorSetHyperlink(self: *Screen) PageList.IncreaseCapacityError!void { error.HyperlinkMapOutOfMemory => { // Attempt to allocate the space that would be required to // insert a new copy of the cursor hyperlink uri in to the - // string alloc, since right now adjustCapacity always just + // string alloc, since right now increaseCapacity always just // adds an extra copy even if one already exists in the page. // If this alloc fails then we know we also need to grow our // string bytes. @@ -9005,214 +8934,6 @@ test "Screen: cursorSetHyperlink OOM + URI too large for string alloc" { try testing.expect(base_string_bytes < s.cursor.page_pin.node.data.capacity.string_bytes); } -test "Screen: adjustCapacity cursor style ref count" { - const testing = std.testing; - const alloc = testing.allocator; - - var s = try init(alloc, .{ .cols = 5, .rows = 5, .max_scrollback = 0 }); - defer s.deinit(); - - try s.setAttribute(.{ .bold = {} }); - try s.testWriteString("1ABCD"); - - { - const page = &s.pages.pages.last.?.data; - try testing.expectEqual( - 6, // All chars + cursor - page.styles.refCount(page.memory, s.cursor.style_id), - ); - } - - // This forces the page to change. - _ = try s.adjustCapacity( - s.cursor.page_pin.node, - .{ .grapheme_bytes = s.cursor.page_pin.node.data.capacity.grapheme_bytes * 2 }, - ); - - // Our ref counts should still be the same - { - const page = &s.pages.pages.last.?.data; - try testing.expectEqual( - 6, // All chars + cursor - page.styles.refCount(page.memory, s.cursor.style_id), - ); - } -} - -test "Screen: adjustCapacity cursor hyperlink exceeds string alloc size" { - const testing = std.testing; - const alloc = testing.allocator; - - var s = try init(alloc, .{ .cols = 80, .rows = 24, .max_scrollback = 0 }); - defer s.deinit(); - - // Start a hyperlink with a URI that just barely fits in the string alloc. - // This will ensure that the redundant copy added in `adjustCapacity` won't - // fit in the available string alloc space. - const uri = "a" ** (pagepkg.std_capacity.string_bytes - 8); - try s.startHyperlink(uri, null); - - // Write some characters with this so that the URI - // is copied to the new page when adjusting capacity. - try s.testWriteString("Hello"); - - // Adjust the capacity, right now this will cause a redundant copy of - // the URI to be added to the string alloc, but since there isn't room - // for this this will clear the cursor hyperlink. - _ = try s.adjustCapacity(s.cursor.page_pin.node, .{}); - - // The cursor hyperlink should have been cleared by the `adjustCapacity` - // call, because there isn't enough room to add the redundant URI string. - // - // This behavior will change, causing this test to fail, if any of these - // changes are made: - // - // - The string alloc is changed to intern strings. - // - // - The adjustCapacity function is changed to ensure the new - // capacity will fit the redundant copy of the hyperlink uri. - // - // - The cursor managed memory handling is reworked so that it - // doesn't reside in the pages anymore and doesn't need this - // accounting. - // - // In such a case, adjust this test accordingly. - try testing.expectEqual(null, s.cursor.hyperlink); - try testing.expectEqual(0, s.cursor.hyperlink_id); -} - -test "Screen: adjustCapacity cursor style exceeds style set capacity" { - const testing = std.testing; - const alloc = testing.allocator; - - var s = try init(alloc, .{ .cols = 80, .rows = 24, .max_scrollback = 1000 }); - defer s.deinit(); - - const page = &s.cursor.page_pin.node.data; - - // We add unique styles to the page until no more will fit. - fill: for (0..255) |bg| { - for (0..255) |fg| { - const st: style.Style = .{ - .bg_color = .{ .palette = @intCast(bg) }, - .fg_color = .{ .palette = @intCast(fg) }, - }; - - s.cursor.style = st; - - // Try to insert the new style, if it doesn't fit then - // we succeeded in filling the style set, so we break. - s.cursor.style_id = page.styles.add( - page.memory, - s.cursor.style, - ) catch break :fill; - - try s.testWriteString("a"); - } - } - - // Adjust the capacity, this should cause the style set to reach the - // same state it was in to begin with, since it will clone the page - // in the same order as the styles were added to begin with, meaning - // the cursor style will not be able to be added to the set, which - // should, right now, result in the cursor style being cleared. - _ = try s.adjustCapacity(s.cursor.page_pin.node, .{}); - - // The cursor style should have been cleared by the `adjustCapacity`. - // - // This behavior will change, causing this test to fail, if either - // of these changes are made: - // - // - The adjustCapacity function is changed to ensure the - // new capacity will definitely fit the cursor style. - // - // - The cursor managed memory handling is reworked so that it - // doesn't reside in the pages anymore and doesn't need this - // accounting. - // - // In such a case, adjust this test accordingly. - try testing.expect(s.cursor.style.default()); - try testing.expectEqual(style.default_id, s.cursor.style_id); -} - -test "Screen: cursorDown to page with insufficient capacity" { - // Regression test for https://github.com/ghostty-org/ghostty/issues/10282 - // - // This test exposes a use-after-realloc bug in cursorDown (and similar - // cursor movement functions). The bug pattern: - // - // 1. cursorDown creates a by-value copy of the pin via page_pin.down(n) - // 2. cursorChangePin is called, which may trigger adjustCapacity - // if the target page's style map is full - // 3. adjustCapacity frees the old page and creates a new one - // 4. The local pin copy still points to the freed page - // 5. rowAndCell() on the stale pin accesses freed memory - - const testing = std.testing; - const alloc = testing.allocator; - - // Small screen to make page boundary crossing easy to set up - var s = try init(alloc, .{ .cols = 10, .rows = 3, .max_scrollback = 1 }); - defer s.deinit(); - - // Scroll down enough to create a second page - const start_page = &s.pages.pages.last.?.data; - const rem = start_page.capacity.rows; - start_page.pauseIntegrityChecks(true); - for (0..rem) |_| try s.cursorDownOrScroll(); - start_page.pauseIntegrityChecks(false); - - // Cursor should now be on a new page - const new_page = &s.cursor.page_pin.node.data; - try testing.expect(start_page != new_page); - - // Fill new_page's style map to capacity. When we move INTO this page - // with a style set, adjustCapacity will be triggered. - { - new_page.pauseIntegrityChecks(true); - defer new_page.pauseIntegrityChecks(false); - defer new_page.assertIntegrity(); - - var n: u24 = 1; - while (new_page.styles.add( - new_page.memory, - .{ .bg_color = .{ .rgb = @bitCast(n) } }, - )) |_| n += 1 else |_| {} - } - - // Move cursor to start of active area and set a style - s.cursorAbsolute(0, 0); - try s.setAttribute(.bold); - try testing.expect(s.cursor.style.flags.bold); - try testing.expect(s.cursor.style_id != style.default_id); - - // Find the row just before the page boundary - for (0..s.pages.rows - 1) |row| { - s.cursorAbsolute(0, @intCast(row)); - const cur_node = s.cursor.page_pin.node; - if (s.cursor.page_pin.down(1)) |next_pin| { - if (next_pin.node != cur_node) { - // Cursor is at 'row', moving down crosses to new_page - try testing.expect(&next_pin.node.data == new_page); - - // This cursorDown triggers the bug: the local page_pin copy - // becomes stale after adjustCapacity, causing rowAndCell() - // to access freed memory. - s.cursorDown(1); - - // If the fix is applied, verify correct state - try testing.expect(s.cursor.y == row + 1); - try testing.expect(s.cursor.style.flags.bold); - - break; - } - } - } else { - // Didn't find boundary - try testing.expect(false); - } -} - test "Screen: increaseCapacity cursor style ref count preserved" { const testing = std.testing; const alloc = testing.allocator; @@ -9448,3 +9169,81 @@ test "Screen: increaseCapacity non-cursor page returns early" { try testing.expect(s.cursor.hyperlink != null); try testing.expectEqualStrings("https://example.com/", s.cursor.hyperlink.?.uri); } + +test "Screen: cursorDown to page with insufficient capacity" { + // Regression test for https://github.com/ghostty-org/ghostty/issues/10282 + // + // This test exposes a use-after-realloc bug in cursorDown (and similar + // cursor movement functions). The bug pattern: + // + // 1. cursorDown creates a by-value copy of the pin via page_pin.down(n) + // 2. cursorChangePin is called, which may trigger adjustCapacity + // if the target page's style map is full + // 3. adjustCapacity frees the old page and creates a new one + // 4. The local pin copy still points to the freed page + // 5. rowAndCell() on the stale pin accesses freed memory + + const testing = std.testing; + const alloc = testing.allocator; + + // Small screen to make page boundary crossing easy to set up + var s = try init(alloc, .{ .cols = 10, .rows = 3, .max_scrollback = 1 }); + defer s.deinit(); + + // Scroll down enough to create a second page + const start_page = &s.pages.pages.last.?.data; + const rem = start_page.capacity.rows; + start_page.pauseIntegrityChecks(true); + for (0..rem) |_| try s.cursorDownOrScroll(); + start_page.pauseIntegrityChecks(false); + + // Cursor should now be on a new page + const new_page = &s.cursor.page_pin.node.data; + try testing.expect(start_page != new_page); + + // Fill new_page's style map to capacity. When we move INTO this page + // with a style set, adjustCapacity will be triggered. + { + new_page.pauseIntegrityChecks(true); + defer new_page.pauseIntegrityChecks(false); + defer new_page.assertIntegrity(); + + var n: u24 = 1; + while (new_page.styles.add( + new_page.memory, + .{ .bg_color = .{ .rgb = @bitCast(n) } }, + )) |_| n += 1 else |_| {} + } + + // Move cursor to start of active area and set a style + s.cursorAbsolute(0, 0); + try s.setAttribute(.bold); + try testing.expect(s.cursor.style.flags.bold); + try testing.expect(s.cursor.style_id != style.default_id); + + // Find the row just before the page boundary + for (0..s.pages.rows - 1) |row| { + s.cursorAbsolute(0, @intCast(row)); + const cur_node = s.cursor.page_pin.node; + if (s.cursor.page_pin.down(1)) |next_pin| { + if (next_pin.node != cur_node) { + // Cursor is at 'row', moving down crosses to new_page + try testing.expect(&next_pin.node.data == new_page); + + // This cursorDown triggers the bug: the local page_pin copy + // becomes stale after adjustCapacity, causing rowAndCell() + // to access freed memory. + s.cursorDown(1); + + // If the fix is applied, verify correct state + try testing.expect(s.cursor.y == row + 1); + try testing.expect(s.cursor.style.flags.bold); + + break; + } + } + } else { + // Didn't find boundary + try testing.expect(false); + } +} diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 310adaf29..3740397d3 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -1669,7 +1669,7 @@ pub fn insertLines(self: *Terminal, count: usize) void { error.OutOfMemory, => @panic("increaseCapacity system allocator OOM"), - // The page can't accomodate the managed memory required + // The page can't accommodate the managed memory required // for this operation. We previously just corrupted // memory here so a crash is better. The right long // term solution is to allocate a new page here From e70452588790f32864400709e38d6eed83966cb1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Jan 2026 13:08:13 -0800 Subject: [PATCH 08/18] terminal: PageList remove adjustCapacity --- src/terminal/PageList.zig | 396 ++++++-------------------------------- 1 file changed, 63 insertions(+), 333 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 9c63fd578..bf2071497 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -1300,31 +1300,30 @@ const ReflowCursor = struct { // If our page can't support an additional cell // with graphemes then we increase capacity. if (self.page.graphemeCount() >= self.page.graphemeCapacity()) { - try self.adjustCapacity(list, .{ - .grapheme_bytes = cap.grapheme_bytes * 2, - }); + try self.increaseCapacity( + list, + .grapheme_bytes, + ); } // Attempt to allocate the space that would be required // for these graphemes, and if it's not available, then - // increase capacity. - if (self.page.grapheme_alloc.alloc( - u21, - self.page.memory, - cps.len, - )) |slice| { - self.page.grapheme_alloc.free(self.page.memory, slice); - } else |_| { - // Grow our capacity until we can - // definitely fit the extra bytes. - const required = cps.len * @sizeOf(u21); - var new_grapheme_capacity: size.GraphemeBytesInt = cap.grapheme_bytes; - while (new_grapheme_capacity - cap.grapheme_bytes < required) { - new_grapheme_capacity *= 2; + // increase capacity. Keep trying until we succeed. + while (true) { + if (self.page.grapheme_alloc.alloc( + u21, + self.page.memory, + cps.len, + )) |slice| { + self.page.grapheme_alloc.free( + self.page.memory, + slice, + ); + break; + } else |_| { + // Grow our capacity until we can fit the extra bytes. + try self.increaseCapacity(list, .grapheme_bytes); } - try self.adjustCapacity(list, .{ - .grapheme_bytes = new_grapheme_capacity, - }); } // This shouldn't fail since we made sure we have space above. @@ -1339,9 +1338,7 @@ const ReflowCursor = struct { // If our page can't support an additional cell // with a hyperlink then we increase capacity. if (self.page.hyperlinkCount() >= self.page.hyperlinkCapacity()) { - try self.adjustCapacity(list, .{ - .hyperlink_bytes = cap.hyperlink_bytes * 2, - }); + try self.increaseCapacity(list, .hyperlink_bytes); } // Ensure that the string alloc has sufficient capacity @@ -1352,23 +1349,26 @@ const ReflowCursor = struct { .explicit => |v| v.len, .implicit => 0, }; - if (self.page.string_alloc.alloc( - u8, - self.page.memory, - additional_required_string_capacity, - )) |slice| { - // We have enough capacity, free the test alloc. - self.page.string_alloc.free(self.page.memory, slice); - } else |_| { - // Grow our capacity until we can - // definitely fit the extra bytes. - var new_string_capacity: size.StringBytesInt = cap.string_bytes; - while (new_string_capacity - cap.string_bytes < additional_required_string_capacity) { - new_string_capacity *= 2; + // Keep trying until we have enough capacity. + while (true) { + if (self.page.string_alloc.alloc( + u8, + self.page.memory, + additional_required_string_capacity, + )) |slice| { + // We have enough capacity, free the test alloc. + self.page.string_alloc.free( + self.page.memory, + slice, + ); + break; + } else |_| { + // Grow our capacity until we can fit the extra bytes. + try self.increaseCapacity( + list, + .string_bytes, + ); } - try self.adjustCapacity(list, .{ - .string_bytes = new_string_capacity, - }); } const dst_id = self.page.hyperlink_set.addWithIdContext( @@ -1380,18 +1380,16 @@ const ReflowCursor = struct { ) catch |err| id: { // If the add failed then either the set needs to grow // or it needs to be rehashed. Either one of those can - // be accomplished by adjusting capacity, either with + // be accomplished by increasing capacity, either with // no actual change or with an increased hyperlink cap. - try self.adjustCapacity(list, switch (err) { - error.OutOfMemory => .{ - .hyperlink_bytes = cap.hyperlink_bytes * 2, - }, - error.NeedsRehash => .{}, + try self.increaseCapacity(list, switch (err) { + error.OutOfMemory => .hyperlink_bytes, + error.NeedsRehash => null, }); // We assume this one will succeed. We dupe the link // again, and don't have to worry about the other one - // because adjusting the capacity naturally clears up + // because increasing the capacity naturally clears up // any managed memory not associated with a cell yet. break :id try self.page.hyperlink_set.addWithIdContext( self.page.memory, @@ -1424,13 +1422,11 @@ const ReflowCursor = struct { ) catch |err| id: { // If the add failed then either the set needs to grow // or it needs to be rehashed. Either one of those can - // be accomplished by adjusting capacity, either with + // be accomplished by increasing capacity, either with // no actual change or with an increased style cap. - try self.adjustCapacity(list, switch (err) { - error.OutOfMemory => .{ - .styles = cap.styles * 2, - }, - error.NeedsRehash => .{}, + try self.increaseCapacity(list, switch (err) { + error.OutOfMemory => .styles, + error.NeedsRehash => null, }); // We assume this one will succeed. @@ -1507,11 +1503,11 @@ const ReflowCursor = struct { } } - /// Adjust the capacity of the current page. - fn adjustCapacity( + /// Increase the capacity of the current page. + fn increaseCapacity( self: *ReflowCursor, list: *PageList, - adjustment: AdjustCapacity, + adjustment: ?IncreaseCapacity, ) !void { const old_x = self.x; const old_y = self.y; @@ -1522,7 +1518,7 @@ const ReflowCursor = struct { // be correct during a reflow. list.pauseIntegrityChecks(true); defer list.pauseIntegrityChecks(false); - break :node try list.adjustCapacity( + break :node try list.increaseCapacity( self.node, adjustment, ); @@ -2642,100 +2638,6 @@ pub fn grow(self: *PageList) !?*List.Node { return next_node; } -/// Adjust the capacity of the given page in the list. -pub const AdjustCapacity = struct { - /// Adjust the number of styles in the page. This may be - /// rounded up if necessary to fit alignment requirements, - /// but it will never be rounded down. - styles: ?size.StyleCountInt = null, - - /// Adjust the number of available grapheme bytes in the page. - grapheme_bytes: ?size.GraphemeBytesInt = null, - - /// Adjust the number of available hyperlink bytes in the page. - hyperlink_bytes: ?size.HyperlinkCountInt = null, - - /// Adjust the number of available string bytes in the page. - string_bytes: ?size.StringBytesInt = null, -}; - -pub const AdjustCapacityError = Allocator.Error || Page.CloneFromError; - -/// Adjust the capacity of the given page in the list. This should -/// be used in cases where OutOfMemory is returned by some operation -/// i.e to increase style counts, grapheme counts, etc. -/// -/// Adjustment works by increasing the capacity of the desired -/// dimension to a certain amount and increases the memory allocation -/// requirement for the backing memory of the page. We currently -/// never split pages or anything like that. Because increased allocation -/// has to happen outside our memory pool, its generally much slower -/// so pages should be sized to be large enough to handle all but -/// exceptional cases. -/// -/// This can currently only INCREASE capacity size. It cannot -/// decrease capacity size. This limitation is only because we haven't -/// yet needed that use case. If we ever do, this can be added. Currently -/// any requests to decrease will be ignored. -pub fn adjustCapacity( - self: *PageList, - node: *List.Node, - adjustment: AdjustCapacity, -) AdjustCapacityError!*List.Node { - defer self.assertIntegrity(); - const page: *Page = &node.data; - - // We always start with the base capacity of the existing page. This - // ensures we never shrink from what we need. - var cap = page.capacity; - - // All ceilPowerOfTwo is unreachable because we're always same or less - // bit width so maxInt is always possible. - if (adjustment.styles) |v| { - const aligned = std.math.ceilPowerOfTwo(size.StyleCountInt, v) catch unreachable; - cap.styles = @max(cap.styles, aligned); - } - if (adjustment.grapheme_bytes) |v| { - const aligned = std.math.ceilPowerOfTwo(size.GraphemeBytesInt, v) catch unreachable; - cap.grapheme_bytes = @max(cap.grapheme_bytes, aligned); - } - if (adjustment.hyperlink_bytes) |v| { - const aligned = std.math.ceilPowerOfTwo(size.HyperlinkCountInt, v) catch unreachable; - cap.hyperlink_bytes = @max(cap.hyperlink_bytes, aligned); - } - if (adjustment.string_bytes) |v| { - const aligned = std.math.ceilPowerOfTwo(size.StringBytesInt, v) catch unreachable; - cap.string_bytes = @max(cap.string_bytes, aligned); - } - - log.info("adjusting page capacity={}", .{cap}); - - // Create our new page and clone the old page into it. - const new_node = try self.createPage(cap); - errdefer self.destroyNode(new_node); - const new_page: *Page = &new_node.data; - assert(new_page.capacity.rows >= page.capacity.rows); - assert(new_page.capacity.cols >= page.capacity.cols); - new_page.size.rows = page.size.rows; - new_page.size.cols = page.size.cols; - try new_page.cloneFrom(page, 0, page.size.rows); - - // Fix up all our tracked pins to point to the new page. - const pin_keys = self.tracked_pins.keys(); - for (pin_keys) |p| { - if (p.node != node) continue; - p.node = new_node; - } - - // Insert this page and destroy the old page - self.pages.insertBefore(node, new_node); - self.pages.remove(node); - self.destroyNode(node); - - new_page.assertIntegrity(); - return new_node; -} - /// Possible dimensions to increase capacity for. pub const IncreaseCapacity = enum { styles, @@ -4991,21 +4893,14 @@ test "PageList grow prune required with a single page" { // behavior during a refactor. This is setting up a scenario that is // possible to trigger a bug (#2280). { - // Adjust our capacity until our page is larger than the standard size. + // Increase our capacity until our page is larger than the standard size. // This is important because it triggers a scenario where our calculated // minSize() which is supposed to accommodate 2 pages is no longer true. - var cap = std_capacity; while (true) { - cap.grapheme_bytes *= 2; - const layout = Page.layout(cap); + const layout = Page.layout(s.pages.first.?.data.capacity); if (layout.total_size > std_size) break; + _ = try s.increaseCapacity(s.pages.first.?, .grapheme_bytes); } - - // Adjust to that capacity. After we should still have one page. - _ = try s.adjustCapacity( - s.pages.first.?, - .{ .grapheme_bytes = cap.grapheme_bytes }, - ); try testing.expect(s.pages.first != null); try testing.expect(s.pages.first == s.pages.last); } @@ -6424,168 +6319,6 @@ test "PageList eraseRowBounded exhausts pages invalidates viewport offset cache" }, s.scrollbar()); } -test "PageList adjustCapacity to increase styles" { - const testing = std.testing; - const alloc = testing.allocator; - - var s = try init(alloc, 2, 2, 0); - defer s.deinit(); - { - try testing.expect(s.pages.first == s.pages.last); - const page = &s.pages.first.?.data; - - // Write all our data so we can assert its the same after - for (0..s.rows) |y| { - for (0..s.cols) |x| { - const rac = page.getRowAndCell(x, y); - rac.cell.* = .{ - .content_tag = .codepoint, - .content = .{ .codepoint = @intCast(x) }, - }; - } - } - } - - // Increase our styles - _ = try s.adjustCapacity( - s.pages.first.?, - .{ .styles = std_capacity.styles * 2 }, - ); - - { - try testing.expect(s.pages.first == s.pages.last); - const page = &s.pages.first.?.data; - for (0..s.rows) |y| { - for (0..s.cols) |x| { - const rac = page.getRowAndCell(x, y); - try testing.expectEqual( - @as(u21, @intCast(x)), - rac.cell.content.codepoint, - ); - } - } - } -} - -test "PageList adjustCapacity to increase graphemes" { - const testing = std.testing; - const alloc = testing.allocator; - - var s = try init(alloc, 2, 2, 0); - defer s.deinit(); - { - try testing.expect(s.pages.first == s.pages.last); - const page = &s.pages.first.?.data; - - // Write all our data so we can assert its the same after - for (0..s.rows) |y| { - for (0..s.cols) |x| { - const rac = page.getRowAndCell(x, y); - rac.cell.* = .{ - .content_tag = .codepoint, - .content = .{ .codepoint = @intCast(x) }, - }; - } - } - } - - // Increase our graphemes - _ = try s.adjustCapacity( - s.pages.first.?, - .{ .grapheme_bytes = std_capacity.grapheme_bytes * 2 }, - ); - - { - try testing.expect(s.pages.first == s.pages.last); - const page = &s.pages.first.?.data; - for (0..s.rows) |y| { - for (0..s.cols) |x| { - const rac = page.getRowAndCell(x, y); - try testing.expectEqual( - @as(u21, @intCast(x)), - rac.cell.content.codepoint, - ); - } - } - } -} - -test "PageList adjustCapacity to increase hyperlinks" { - const testing = std.testing; - const alloc = testing.allocator; - - var s = try init(alloc, 2, 2, 0); - defer s.deinit(); - { - try testing.expect(s.pages.first == s.pages.last); - const page = &s.pages.first.?.data; - - // Write all our data so we can assert its the same after - for (0..s.rows) |y| { - for (0..s.cols) |x| { - const rac = page.getRowAndCell(x, y); - rac.cell.* = .{ - .content_tag = .codepoint, - .content = .{ .codepoint = @intCast(x) }, - }; - } - } - } - - // Increase our graphemes - _ = try s.adjustCapacity( - s.pages.first.?, - .{ .hyperlink_bytes = @max(std_capacity.hyperlink_bytes * 2, 2048) }, - ); - - { - try testing.expect(s.pages.first == s.pages.last); - const page = &s.pages.first.?.data; - for (0..s.rows) |y| { - for (0..s.cols) |x| { - const rac = page.getRowAndCell(x, y); - try testing.expectEqual( - @as(u21, @intCast(x)), - rac.cell.content.codepoint, - ); - } - } - } -} - -test "PageList adjustCapacity after col shrink" { - const testing = std.testing; - const alloc = testing.allocator; - - var s = try init(alloc, 10, 2, 0); - defer s.deinit(); - - // Shrink columns - this updates size.cols but not capacity.cols - try s.resize(.{ .cols = 5, .reflow = false }); - try testing.expectEqual(5, s.cols); - - { - const page = &s.pages.first.?.data; - // capacity.cols is still 10, but size.cols should be 5 - try testing.expectEqual(5, page.size.cols); - try testing.expect(page.capacity.cols >= 10); - } - - // Now adjust capacity (e.g., to increase styles) - // This should preserve the current size.cols, not revert to capacity.cols - _ = try s.adjustCapacity( - s.pages.first.?, - .{ .styles = std_capacity.styles * 2 }, - ); - - { - const page = &s.pages.first.?.data; - // After adjustCapacity, size.cols should still be 5, not 10 - try testing.expectEqual(5, page.size.cols); - try testing.expectEqual(5, s.cols); - } -} - test "PageList increaseCapacity to increase styles" { const testing = std.testing; const alloc = testing.allocator; @@ -6799,13 +6532,12 @@ test "PageList increaseCapacity returns OutOfSpace at max capacity" { var s = try init(alloc, 2, 2, 0); defer s.deinit(); - // Set styles capacity to a value that will overflow when doubled - // We need to create a page with capacity at more than half of max + // Keep increasing styles capacity until we're at more than half of max const max_styles = std.math.maxInt(size.StyleCountInt); const half_max = max_styles / 2 + 1; - - // Adjust capacity to near-max first - _ = try s.adjustCapacity(s.pages.first.?, .{ .styles = half_max }); + while (s.pages.first.?.data.capacity.styles < half_max) { + _ = try s.increaseCapacity(s.pages.first.?, .styles); + } // Now increaseCapacity should fail with OutOfSpace try testing.expectError( @@ -11485,12 +11217,10 @@ test "PageList grow reuses non-standard page without leak" { var s = try init(alloc, 80, 24, 3 * std_size); defer s.deinit(); - // Save the first page node before adjustment - const first_before = s.pages.first.?; - - // Adjust the first page to have non-standard capacity. We use a small - // increase that makes it just slightly larger than std_size. - _ = try s.adjustCapacity(first_before, .{ .grapheme_bytes = std_size + 1 }); + // Increase the first page capacity to make it non-standard (larger than std_size). + while (s.pages.first.?.data.memory.len <= std_size) { + _ = try s.increaseCapacity(s.pages.first.?, .grapheme_bytes); + } // The first page should now have non-standard memory size. try testing.expect(s.pages.first.?.data.memory.len > std_size); From 6b2455828e1aba8898d3c11fc5b9e06f78a1b91c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Jan 2026 15:26:12 -0800 Subject: [PATCH 09/18] terminal: resizeWithoutReflowGrowCols can only fail for OOM --- src/terminal/PageList.zig | 89 +++++++++++++++++++++++++++++++++------ 1 file changed, 75 insertions(+), 14 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index bf2071497..043a8f191 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -1652,7 +1652,7 @@ const ReflowCursor = struct { } }; -fn resizeWithoutReflow(self: *PageList, opts: Resize) !void { +fn resizeWithoutReflow(self: *PageList, opts: Resize) Allocator.Error!void { // We only set the new min_max_size if we're not reflowing. If we are // reflowing, then resize handles this for us. const old_min_max_size = self.min_max_size; @@ -1812,26 +1812,44 @@ fn resizeWithoutReflowGrowCols( self: *PageList, cols: size.CellCountInt, chunk: PageIterator.Chunk, -) !void { +) Allocator.Error!void { assert(cols > self.cols); const page = &chunk.node.data; - const cap = try page.capacity.adjust(.{ .cols = cols }); // Update our col count const old_cols = self.cols; - self.cols = cap.cols; + self.cols = cols; errdefer self.cols = old_cols; // Unlikely fast path: we have capacity in the page. This // is only true if we resized to less cols earlier. - if (page.capacity.cols >= cap.cols) { - page.size.cols = cap.cols; + if (page.capacity.cols >= cols) { + page.size.cols = cols; return; } // Likely slow path: we don't have capacity, so we need // to allocate a page, and copy the old data into it. + // Try to fit our new column size into our existing page capacity. + // If that doesn't work then use a non-standard page with the + // given columns. + const cap = page.capacity.adjust( + .{ .cols = cols }, + ) catch |err| err: { + comptime assert(@TypeOf(err) == error{OutOfMemory}); + + // We verify all maxed out page layouts work. + var cap = page.capacity; + cap.cols = cols; + + // We're growing columns so we can only get less rows so use + // the lesser of our capacity and size so we minimize wasted + // rows. + cap.rows = @min(page.size.rows, cap.rows); + break :err cap; + }; + // On error, we need to undo all the pages we've added. const prev = chunk.node.prev; errdefer { @@ -1895,6 +1913,31 @@ fn resizeWithoutReflowGrowCols( assert(prev_page.size.rows <= prev_page.capacity.rows); } + // If we have an error, we clear the rows we just added to our prev page. + const prev_copied = copied; + errdefer if (prev_copied > 0) { + const prev_page = &prev.?.data; + const prev_size = prev_page.size.rows - prev_copied; + const prev_rows = prev_page.rows.ptr(prev_page.memory)[prev_size..prev_page.size.rows]; + for (prev_rows) |*row| prev_page.clearCells( + row, + 0, + prev_page.size.cols, + ); + prev_page.size.rows = prev_size; + }; + + // We delete any of the nodes we added. + errdefer { + var it = chunk.node.prev; + while (it) |node| { + if (node == prev) break; + it = node.prev; + self.pages.remove(node); + self.destroyNode(node); + } + } + // We need to loop because our col growth may force us // to split pages. while (copied < page.size.rows) { @@ -1907,19 +1950,33 @@ fn resizeWithoutReflowGrowCols( // Perform the copy const y_start = copied; - const y_end = copied + len; - const src_rows = page.rows.ptr(page.memory)[y_start..y_end]; + const src_rows = page.rows.ptr(page.memory)[y_start .. copied + len]; const dst_rows = new_node.data.rows.ptr(new_node.data.memory)[0..len]; for (dst_rows, src_rows) |*dst_row, *src_row| { new_node.data.size.rows += 1; - errdefer new_node.data.size.rows -= 1; - try new_node.data.cloneRowFrom( + if (new_node.data.cloneRowFrom( page, dst_row, src_row, - ); + )) |_| { + copied += 1; + } else |err| { + // I don't THINK this should be possible, because while our + // row count may diminish due to the adjustment, our + // prior capacity should have been sufficient to hold all the + // managed memory. + log.warn( + "unexpected cloneRowFrom failure during resizeWithoutReflowGrowCols: {}", + .{err}, + ); + + // We can actually safely handle this though by exiting + // this loop early and cutting our copy short. + new_node.data.size.rows -= 1; + break; + } } - copied = y_end; + const y_end = copied; // Insert our new page self.pages.insertBefore(chunk.node, new_node); @@ -1936,6 +1993,10 @@ fn resizeWithoutReflowGrowCols( } assert(copied == page.size.rows); + // Our prior errdeferes are invalid after this point so ensure + // we don't have any more errors. + errdefer comptime unreachable; + // Remove the old page. // Deallocate the old page. self.pages.remove(chunk.node); @@ -2514,7 +2575,7 @@ inline fn growRequiredForActive(self: *const PageList) bool { /// adhere to max_size. /// /// This returns the newly allocated page node if there is one. -pub fn grow(self: *PageList) !?*List.Node { +pub fn grow(self: *PageList) Allocator.Error!?*List.Node { defer self.assertIntegrity(); const last = self.pages.last.?; @@ -2533,7 +2594,7 @@ pub fn grow(self: *PageList) !?*List.Node { // Get the layout first so our failable work is done early. // We'll need this for both paths. - const cap = try std_capacity.adjust(.{ .cols = self.cols }); + const cap = initialCapacity(self.cols); // If allocation would exceed our max size, we prune the first page. // We don't need to reallocate because we can simply reuse that first From d626984418cf91910543683f64ba95d24815f4ca Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Jan 2026 20:31:22 -0800 Subject: [PATCH 10/18] terminal: reflowCursor improve error handling on assumed cases --- src/terminal/PageList.zig | 233 +++++++++++++++++++++++++++++++------- 1 file changed, 190 insertions(+), 43 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 043a8f191..755e0a393 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -992,32 +992,71 @@ fn resizeCols( } else null; defer if (preserved_cursor) |c| self.untrackPin(c.tracked_pin); - const first = self.pages.first.?; - var it = self.rowIterator(.right_down, .{ .screen = .{} }, null); + // Create the first node that contains our reflow. + const first_rewritten_node = node: { + const page = &self.pages.first.?.data; + const cap = page.capacity.adjust( + .{ .cols = cols }, + ) catch |err| err: { + comptime assert(@TypeOf(err) == error{OutOfMemory}); - const dst_node = try self.createPage(try first.data.capacity.adjust(.{ .cols = cols })); - dst_node.data.size.rows = 1; + // We verify all maxed out page layouts work. + var cap = page.capacity; + cap.cols = cols; + + // We're growing columns so we can only get less rows so use + // the lesser of our capacity and size so we minimize wasted + // rows. + cap.rows = @min(page.size.rows, cap.rows); + break :err cap; + }; + + const node = try self.createPage(cap); + node.data.size.rows = 1; + break :node node; + }; + + // We need to grab our rowIterator now before we rewrite our + // linked list below. + var it = self.rowIterator( + .right_down, + .{ .screen = .{} }, + null, + ); + errdefer { + // If an error occurs, we're in a pretty disastrous broken state, + // but we should still try to clean up our leaked memory. Free + // any of the remaining orphaned pages from before. If we reflowed + // successfully this will be null. + var node_: ?*Node = if (it.chunk) |chunk| chunk.node else null; + while (node_) |node| { + node_ = node.next; + self.destroyNode(node); + } + } // Set our new page as the only page. This orphans the existing pages // in the list, but that's fine since we're gonna delete them anyway. - self.pages.first = dst_node; - self.pages.last = dst_node; + self.pages.first = first_rewritten_node; + self.pages.last = first_rewritten_node; // Reflow all our rows. { - var dst_cursor = ReflowCursor.init(dst_node); + var reflow_cursor: ReflowCursor = .init(first_rewritten_node); while (it.next()) |row| { - try dst_cursor.reflowRow(self, row); + try reflow_cursor.reflowRow(self, row); - // Once we're done reflowing a page, destroy it. + // Once we're done reflowing a page, destroy it immediately. + // This frees memory and makes it more likely in memory + // constrained environments that the next reflow will work. if (row.y == row.node.data.size.rows - 1) { self.destroyNode(row.node); } } // At the end of the reflow, setup our total row cache - // log.warn("total old={} new={}", .{ self.total_rows, dst_cursor.total_rows }); - self.total_rows = dst_cursor.total_rows; + // log.warn("total old={} new={}", .{ self.total_rows, reflow_cursor.total_rows }); + self.total_rows = reflow_cursor.total_rows; } // If our total rows is less than our active rows, we need to grow. @@ -1114,16 +1153,11 @@ const ReflowCursor = struct { const src_page: *Page = &row.node.data; const src_row = row.rowAndCell().row; const src_y = row.y; - - // Inherit increased styles or grapheme bytes from - // the src page we're reflowing from for new pages. - const cap = try src_page.capacity.adjust(.{ .cols = self.page.size.cols }); - const cells = src_row.cells.ptr(src_page.memory)[0..src_page.size.cols]; + // Calculate the columns in this row. First up we trim non-semantic + // rightmost blanks. var cols_len = src_page.size.cols; - - // If the row is wrapped, all empty cells are meaningful. if (!src_row.wrap) { while (cols_len > 0) { if (!cells[cols_len - 1].isEmpty()) break; @@ -1145,9 +1179,10 @@ const ReflowCursor = struct { // If this pin is in the blanks on the right and past the end // of the dst col width then we move it to the end of the dst // col width instead. - if (p.x >= cols_len) { - p.x = @min(p.x, cap.cols - 1 - self.x); - } + if (p.x >= cols_len) p.x = @min( + p.x, + self.page.size.cols - 1 - self.x, + ); // We increase our col len to at least include this pin. // This ensures that blank rows with pins are processed, @@ -1162,16 +1197,29 @@ const ReflowCursor = struct { // If this blank row was a wrap continuation somehow // then we won't need to write it since it should be // a part of the previously written row. - if (!src_row.wrap_continuation) { - self.new_rows += 1; - } + if (!src_row.wrap_continuation) self.new_rows += 1; return; } + // Inherit increased styles or grapheme bytes from the src page + // we're reflowing from for new pages. + const cap = src_page.capacity.adjust( + .{ .cols = self.page.size.cols }, + ) catch |err| err: { + comptime assert(@TypeOf(err) == error{OutOfMemory}); + + var cap = src_page.capacity; + cap.cols = self.page.size.cols; + // We're already a non-standard page. We don't want to + // inherit a massive set of rows, so cap it at our std size. + cap.rows = @min(src_page.size.rows, std_capacity.rows); + break :err cap; + }; + // Our row isn't blank, write any new rows we deferred. while (self.new_rows > 0) { - self.new_rows -= 1; try self.cursorScrollOrNewPage(list, cap); + self.new_rows -= 1; } self.copyRowMetadata(src_row); @@ -1326,12 +1374,30 @@ const ReflowCursor = struct { } } - // This shouldn't fail since we made sure we have space above. - try self.page.setGraphemes(self.page_row, self.page_cell, cps); + self.page.setGraphemes( + self.page_row, + self.page_cell, + cps, + ) catch |err| { + // This shouldn't fail since we made sure we have space + // above. There is no reasonable behavior we can take here + // so we have a warn level log. This is ALMOST non-recoverable, + // though we choose to recover by corrupting the cell + // to a non-grapheme codepoint. + log.err("setGraphemes failed after capacity increase err={}", .{err}); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + // Unsafe builds we throw away grapheme data! + cell.content_tag = .codepoint; + cell.content = .{ .codepoint = 0xFFFD }; + }; } // Copy hyperlink data. - if (cell.hyperlink) { + if (cell.hyperlink) hyperlink: { const src_id = src_page.lookupHyperlink(cell).?; const src_link = src_page.hyperlink_set.get(src_page.memory, src_id); @@ -1371,10 +1437,25 @@ const ReflowCursor = struct { } } + const dst_link = src_link.dupe( + src_page, + self.page, + ) catch |err| { + // This shouldn't fail since we did a capacity + // check above. + log.err("link dupe failed with capacity check err={}", .{err}); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + cell.hyperlink = false; + break :hyperlink; + }; + const dst_id = self.page.hyperlink_set.addWithIdContext( self.page.memory, - // We made sure there was enough capacity for this above. - try src_link.dupe(src_page, self.page), + dst_link, src_id, .{ .page = self.page }, ) catch |err| id: { @@ -1387,29 +1468,80 @@ const ReflowCursor = struct { error.NeedsRehash => null, }); + // We dupe the link again, and don't have to worry about + // freeing the other one because increasing the capacity + // destroyed the prior page. + const dst_link2 = src_link.dupe( + src_page, + self.page, + ) catch |err2| { + // This shouldn't fail since we did a capacity + // check above. + log.err("link dupe failed with capacity check err={}", .{err2}); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + cell.hyperlink = false; + break :hyperlink; + }; + // We assume this one will succeed. We dupe the link // again, and don't have to worry about the other one // because increasing the capacity naturally clears up // any managed memory not associated with a cell yet. - break :id try self.page.hyperlink_set.addWithIdContext( + break :id self.page.hyperlink_set.addWithIdContext( self.page.memory, - try src_link.dupe(src_page, self.page), + dst_link2, src_id, .{ .page = self.page }, - ); + ) catch |err2| { + // This shouldn't happen since we increased capacity + // above so we handle it like the other similar + // cases and log it, crash in safe builds, and + // remove the hyperlink in unsafe builds. + log.err( + "addWithIdContext failed after capacity increase err={}", + .{err2}, + ); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + dst_link2.free(self.page); + cell.hyperlink = false; + break :hyperlink; + }; } orelse src_id; - // We expect this to succeed due to the - // hyperlinkCapacity check we did before. - try self.page.setHyperlink( + // We expect this to succeed due to the hyperlinkCapacity + // check we did before. If it doesn't succeed let's + // log it, crash (in safe builds), and clear our state. + self.page.setHyperlink( self.page_row, self.page_cell, dst_id, - ); + ) catch |err| { + log.err( + "setHyperlink failed after capacity increase err={}", + .{err}, + ); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + // Unsafe builds we throw away hyperlink data! + self.page.hyperlink_set.release(self.page.memory, dst_id); + cell.hyperlink = false; + break :hyperlink; + }; } // Copy style data. - if (cell.hasStyling()) { + if (cell.hasStyling()) style: { const style = src_page.styles.get( src_page.memory, cell.style_id, @@ -1430,15 +1562,29 @@ const ReflowCursor = struct { }); // We assume this one will succeed. - break :id try self.page.styles.addWithId( + break :id self.page.styles.addWithId( self.page.memory, style, cell.style_id, - ); + ) catch |err2| { + // Should not fail since we just modified capacity + // above. Log it, crash in safe builds, clear style + // in unsafe builds. + log.err( + "addWithId failed after capacity increase err={}", + .{err2}, + ); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + cell.style_id = stylepkg.default_id; + break :style; + }; } orelse cell.style_id; self.page_row.styled = true; - self.page_cell.style_id = id; } @@ -1574,12 +1720,13 @@ const ReflowCursor = struct { self: *ReflowCursor, list: *PageList, cap: Capacity, - ) !void { + ) Allocator.Error!void { // Remember our new row count so we can restore it // after reinitializing our cursor on the new page. const new_rows = self.new_rows; const node = try list.createPage(cap); + errdefer comptime unreachable; node.data.size.rows = 1; list.pages.insertAfter(self.node, node); @@ -1594,7 +1741,7 @@ const ReflowCursor = struct { self: *ReflowCursor, list: *PageList, cap: Capacity, - ) !void { + ) Allocator.Error!void { // The functions below may overwrite self so we need to cache // our total rows. We add one because no matter what when this // returns we'll have one more row added. From 25643ec806e5be828b236b8ead5814a0fcabaff8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Jan 2026 06:22:05 -0800 Subject: [PATCH 11/18] terminal: reflowRow extract writeCell --- src/terminal/PageList.zig | 733 ++++++++++++++++++++------------------ 1 file changed, 384 insertions(+), 349 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 755e0a393..0ca00f45c 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -1247,355 +1247,19 @@ const ReflowCursor = struct { } } - const cell = &cells[x]; - x += 1; - - // Copy cell contents. - switch (cell.content_tag) { - .codepoint, - .codepoint_grapheme, - => switch (cell.wide) { - .narrow => self.page_cell.* = cell.*, - - .wide => if (self.page.size.cols > 1) { - if (self.x == self.page.size.cols - 1) { - // If there's a wide character in the last column of - // the reflowed page then we need to insert a spacer - // head and wrap before handling it. - self.page_cell.* = .{ - .content_tag = .codepoint, - .content = .{ .codepoint = 0 }, - .wide = .spacer_head, - }; - - // Decrement the source position so that when we - // loop we'll process this source cell again, - // since we can't copy it into a spacer head. - x -= 1; - - // Move to the next row (this sets pending wrap - // which will cause us to wrap on the next - // iteration). - self.cursorForward(); - continue; - } else { - self.page_cell.* = cell.*; - } - } else { - // Edge case, when resizing to 1 column, wide - // characters are just destroyed and replaced - // with empty narrow cells. - self.page_cell.content.codepoint = 0; - self.page_cell.wide = .narrow; - self.cursorForward(); - // Skip spacer tail so it doesn't cause a wrap. - x += 1; - continue; - }, - - .spacer_tail => if (self.page.size.cols > 1) { - self.page_cell.* = cell.*; - } else { - // Edge case, when resizing to 1 column, wide - // characters are just destroyed and replaced - // with empty narrow cells, so we should just - // discard any spacer tails. - continue; - }, - - .spacer_head => { - // Spacer heads should be ignored. If we need a - // spacer head in our reflowed page, it is added - // when processing the wide cell it belongs to. - continue; - }, - }, - - .bg_color_palette, - .bg_color_rgb, - => { - // These are guaranteed to have no style or grapheme - // data associated with them so we can fast path them. - self.page_cell.* = cell.*; - self.cursorForward(); - continue; - }, + switch (try self.writeCell( + list, + &cells[x], + src_page, + )) { + // Wrote the cell, move to the next. + .success => x += 1, + // Wrote the cell but request to skip the next so skip it. + // This is used for things like spacers. + .skip_next => x += 2, + // Didn't write the cell, repeat writing this same cell. + .repeat => {}, } - - // These will create issues by trying to clone managed memory that - // isn't set if the current dst row needs to be moved to a new page. - // They'll be fixed once we do properly copy the relevant memory. - self.page_cell.content_tag = .codepoint; - self.page_cell.hyperlink = false; - self.page_cell.style_id = stylepkg.default_id; - - // std.log.warn("\nsrc_y={} src_x={} dst_y={} dst_x={} dst_cols={} cp={X} wide={} page_cell_wide={}", .{ - // src_y, - // x, - // self.y, - // self.x, - // self.page.size.cols, - // cell.content.codepoint, - // cell.wide, - // self.page_cell.wide, - // }); - - // Copy grapheme data. - if (cell.content_tag == .codepoint_grapheme) { - // Copy the graphemes - const cps = src_page.lookupGrapheme(cell).?; - - // If our page can't support an additional cell - // with graphemes then we increase capacity. - if (self.page.graphemeCount() >= self.page.graphemeCapacity()) { - try self.increaseCapacity( - list, - .grapheme_bytes, - ); - } - - // Attempt to allocate the space that would be required - // for these graphemes, and if it's not available, then - // increase capacity. Keep trying until we succeed. - while (true) { - if (self.page.grapheme_alloc.alloc( - u21, - self.page.memory, - cps.len, - )) |slice| { - self.page.grapheme_alloc.free( - self.page.memory, - slice, - ); - break; - } else |_| { - // Grow our capacity until we can fit the extra bytes. - try self.increaseCapacity(list, .grapheme_bytes); - } - } - - self.page.setGraphemes( - self.page_row, - self.page_cell, - cps, - ) catch |err| { - // This shouldn't fail since we made sure we have space - // above. There is no reasonable behavior we can take here - // so we have a warn level log. This is ALMOST non-recoverable, - // though we choose to recover by corrupting the cell - // to a non-grapheme codepoint. - log.err("setGraphemes failed after capacity increase err={}", .{err}); - if (comptime std.debug.runtime_safety) { - // Force a crash with safe builds. - unreachable; - } - - // Unsafe builds we throw away grapheme data! - cell.content_tag = .codepoint; - cell.content = .{ .codepoint = 0xFFFD }; - }; - } - - // Copy hyperlink data. - if (cell.hyperlink) hyperlink: { - const src_id = src_page.lookupHyperlink(cell).?; - const src_link = src_page.hyperlink_set.get(src_page.memory, src_id); - - // If our page can't support an additional cell - // with a hyperlink then we increase capacity. - if (self.page.hyperlinkCount() >= self.page.hyperlinkCapacity()) { - try self.increaseCapacity(list, .hyperlink_bytes); - } - - // Ensure that the string alloc has sufficient capacity - // to dupe the link (and the ID if it's not implicit). - const additional_required_string_capacity = - src_link.uri.len + - switch (src_link.id) { - .explicit => |v| v.len, - .implicit => 0, - }; - // Keep trying until we have enough capacity. - while (true) { - if (self.page.string_alloc.alloc( - u8, - self.page.memory, - additional_required_string_capacity, - )) |slice| { - // We have enough capacity, free the test alloc. - self.page.string_alloc.free( - self.page.memory, - slice, - ); - break; - } else |_| { - // Grow our capacity until we can fit the extra bytes. - try self.increaseCapacity( - list, - .string_bytes, - ); - } - } - - const dst_link = src_link.dupe( - src_page, - self.page, - ) catch |err| { - // This shouldn't fail since we did a capacity - // check above. - log.err("link dupe failed with capacity check err={}", .{err}); - if (comptime std.debug.runtime_safety) { - // Force a crash with safe builds. - unreachable; - } - - cell.hyperlink = false; - break :hyperlink; - }; - - const dst_id = self.page.hyperlink_set.addWithIdContext( - self.page.memory, - dst_link, - src_id, - .{ .page = self.page }, - ) catch |err| id: { - // If the add failed then either the set needs to grow - // or it needs to be rehashed. Either one of those can - // be accomplished by increasing capacity, either with - // no actual change or with an increased hyperlink cap. - try self.increaseCapacity(list, switch (err) { - error.OutOfMemory => .hyperlink_bytes, - error.NeedsRehash => null, - }); - - // We dupe the link again, and don't have to worry about - // freeing the other one because increasing the capacity - // destroyed the prior page. - const dst_link2 = src_link.dupe( - src_page, - self.page, - ) catch |err2| { - // This shouldn't fail since we did a capacity - // check above. - log.err("link dupe failed with capacity check err={}", .{err2}); - if (comptime std.debug.runtime_safety) { - // Force a crash with safe builds. - unreachable; - } - - cell.hyperlink = false; - break :hyperlink; - }; - - // We assume this one will succeed. We dupe the link - // again, and don't have to worry about the other one - // because increasing the capacity naturally clears up - // any managed memory not associated with a cell yet. - break :id self.page.hyperlink_set.addWithIdContext( - self.page.memory, - dst_link2, - src_id, - .{ .page = self.page }, - ) catch |err2| { - // This shouldn't happen since we increased capacity - // above so we handle it like the other similar - // cases and log it, crash in safe builds, and - // remove the hyperlink in unsafe builds. - log.err( - "addWithIdContext failed after capacity increase err={}", - .{err2}, - ); - if (comptime std.debug.runtime_safety) { - // Force a crash with safe builds. - unreachable; - } - - dst_link2.free(self.page); - cell.hyperlink = false; - break :hyperlink; - }; - } orelse src_id; - - // We expect this to succeed due to the hyperlinkCapacity - // check we did before. If it doesn't succeed let's - // log it, crash (in safe builds), and clear our state. - self.page.setHyperlink( - self.page_row, - self.page_cell, - dst_id, - ) catch |err| { - log.err( - "setHyperlink failed after capacity increase err={}", - .{err}, - ); - if (comptime std.debug.runtime_safety) { - // Force a crash with safe builds. - unreachable; - } - - // Unsafe builds we throw away hyperlink data! - self.page.hyperlink_set.release(self.page.memory, dst_id); - cell.hyperlink = false; - break :hyperlink; - }; - } - - // Copy style data. - if (cell.hasStyling()) style: { - const style = src_page.styles.get( - src_page.memory, - cell.style_id, - ).*; - - const id = self.page.styles.addWithId( - self.page.memory, - style, - cell.style_id, - ) catch |err| id: { - // If the add failed then either the set needs to grow - // or it needs to be rehashed. Either one of those can - // be accomplished by increasing capacity, either with - // no actual change or with an increased style cap. - try self.increaseCapacity(list, switch (err) { - error.OutOfMemory => .styles, - error.NeedsRehash => null, - }); - - // We assume this one will succeed. - break :id self.page.styles.addWithId( - self.page.memory, - style, - cell.style_id, - ) catch |err2| { - // Should not fail since we just modified capacity - // above. Log it, crash in safe builds, clear style - // in unsafe builds. - log.err( - "addWithId failed after capacity increase err={}", - .{err2}, - ); - if (comptime std.debug.runtime_safety) { - // Force a crash with safe builds. - unreachable; - } - - cell.style_id = stylepkg.default_id; - break :style; - }; - } orelse cell.style_id; - - self.page_row.styled = true; - self.page_cell.style_id = id; - } - - if (comptime build_options.kitty_graphics) { - // Copy Kitty virtual placeholder status - if (cell.codepoint() == kitty.graphics.unicode.placeholder) { - self.page_row.kitty_virtual_placeholder = true; - } - } - - self.cursorForward(); } // If the source row isn't wrapped then we should scroll afterwards. @@ -1604,6 +1268,377 @@ const ReflowCursor = struct { } } + /// Write a cell. On error, this will not unwrite the cell but + /// the cell may be incomplete (but valid). For example, if the source + /// cell is styled and we failed to allocate space for styles, the + /// written cell may not be styled but it is valid. + /// + /// The key failure to recognize for callers is when we can't increase + /// capacity in our destination page. In this case, the caller may want + /// to split the page at this row, rewrite the row into a new page + /// and continue from there. + /// + /// But this function guarantees the terminal/page will be in a + /// coherent state even on error. + fn writeCell( + self: *ReflowCursor, + list: *PageList, + cell: *const pagepkg.Cell, + src_page: *const Page, + ) IncreaseCapacityError!enum { + success, + repeat, + skip_next, + } { + // Copy cell contents. + switch (cell.content_tag) { + .codepoint, + .codepoint_grapheme, + => switch (cell.wide) { + .narrow => self.page_cell.* = cell.*, + + .wide => if (self.page.size.cols > 1) { + if (self.x == self.page.size.cols - 1) { + // If there's a wide character in the last column of + // the reflowed page then we need to insert a spacer + // head and wrap before handling it. + self.page_cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = 0 }, + .wide = .spacer_head, + }; + + // Move to the next row (this sets pending wrap + // which will cause us to wrap on the next + // iteration). + self.cursorForward(); + + // Decrement the source position so that when we + // loop we'll process this source cell again, + // since we can't copy it into a spacer head. + return .repeat; + } else { + self.page_cell.* = cell.*; + } + } else { + // Edge case, when resizing to 1 column, wide + // characters are just destroyed and replaced + // with empty narrow cells. + self.page_cell.content.codepoint = 0; + self.page_cell.wide = .narrow; + self.cursorForward(); + + // Skip spacer tail so it doesn't cause a wrap. + return .skip_next; + }, + + .spacer_tail => if (self.page.size.cols > 1) { + self.page_cell.* = cell.*; + } else { + // Edge case, when resizing to 1 column, wide + // characters are just destroyed and replaced + // with empty narrow cells, so we should just + // discard any spacer tails. + return .success; + }, + + .spacer_head => { + // Spacer heads should be ignored. If we need a + // spacer head in our reflowed page, it is added + // when processing the wide cell it belongs to. + return .success; + }, + }, + + .bg_color_palette, + .bg_color_rgb, + => { + // These are guaranteed to have no style or grapheme + // data associated with them so we can fast path them. + self.page_cell.* = cell.*; + self.cursorForward(); + return .success; + }, + } + + // These will create issues by trying to clone managed memory that + // isn't set if the current dst row needs to be moved to a new page. + // They'll be fixed once we do properly copy the relevant memory. + self.page_cell.content_tag = .codepoint; + self.page_cell.hyperlink = false; + self.page_cell.style_id = stylepkg.default_id; + + // std.log.warn("\nsrc_y={} src_x={} dst_y={} dst_x={} dst_cols={} cp={X} wide={} page_cell_wide={}", .{ + // src_y, + // x, + // self.y, + // self.x, + // self.page.size.cols, + // cell.content.codepoint, + // cell.wide, + // self.page_cell.wide, + // }); + + if (comptime build_options.kitty_graphics) { + // Copy Kitty virtual placeholder status + if (cell.codepoint() == kitty.graphics.unicode.placeholder) { + self.page_row.kitty_virtual_placeholder = true; + } + } + + // Copy grapheme data. + if (cell.content_tag == .codepoint_grapheme) { + // Copy the graphemes + const cps = src_page.lookupGrapheme(cell).?; + + // If our page can't support an additional cell + // with graphemes then we increase capacity. + if (self.page.graphemeCount() >= self.page.graphemeCapacity()) { + try self.increaseCapacity( + list, + .grapheme_bytes, + ); + } + + // Attempt to allocate the space that would be required + // for these graphemes, and if it's not available, then + // increase capacity. Keep trying until we succeed. + while (true) { + if (self.page.grapheme_alloc.alloc( + u21, + self.page.memory, + cps.len, + )) |slice| { + self.page.grapheme_alloc.free( + self.page.memory, + slice, + ); + break; + } else |_| { + // Grow our capacity until we can fit the extra bytes. + try self.increaseCapacity(list, .grapheme_bytes); + } + } + + self.page.setGraphemes( + self.page_row, + self.page_cell, + cps, + ) catch |err| { + // This shouldn't fail since we made sure we have space + // above. There is no reasonable behavior we can take here + // so we have a warn level log. This is ALMOST non-recoverable, + // though we choose to recover by corrupting the cell + // to a non-grapheme codepoint. + log.err("setGraphemes failed after capacity increase err={}", .{err}); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + // Unsafe builds we throw away grapheme data! + cell.content_tag = .codepoint; + cell.content = .{ .codepoint = 0xFFFD }; + }; + } + + // Copy hyperlink data. + if (cell.hyperlink) hyperlink: { + const src_id = src_page.lookupHyperlink(cell).?; + const src_link = src_page.hyperlink_set.get(src_page.memory, src_id); + + // If our page can't support an additional cell + // with a hyperlink then we increase capacity. + if (self.page.hyperlinkCount() >= self.page.hyperlinkCapacity()) { + try self.increaseCapacity(list, .hyperlink_bytes); + } + + // Ensure that the string alloc has sufficient capacity + // to dupe the link (and the ID if it's not implicit). + const additional_required_string_capacity = + src_link.uri.len + + switch (src_link.id) { + .explicit => |v| v.len, + .implicit => 0, + }; + // Keep trying until we have enough capacity. + while (true) { + if (self.page.string_alloc.alloc( + u8, + self.page.memory, + additional_required_string_capacity, + )) |slice| { + // We have enough capacity, free the test alloc. + self.page.string_alloc.free( + self.page.memory, + slice, + ); + break; + } else |_| { + // Grow our capacity until we can fit the extra bytes. + try self.increaseCapacity( + list, + .string_bytes, + ); + } + } + + const dst_link = src_link.dupe( + src_page, + self.page, + ) catch |err| { + // This shouldn't fail since we did a capacity + // check above. + log.err("link dupe failed with capacity check err={}", .{err}); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + break :hyperlink; + }; + + const dst_id = self.page.hyperlink_set.addWithIdContext( + self.page.memory, + dst_link, + src_id, + .{ .page = self.page }, + ) catch |err| id: { + // Always free our original link in case the increaseCap + // call fails so we aren't leaking memory. + dst_link.free(self.page); + + // If the add failed then either the set needs to grow + // or it needs to be rehashed. Either one of those can + // be accomplished by increasing capacity, either with + // no actual change or with an increased hyperlink cap. + try self.increaseCapacity(list, switch (err) { + error.OutOfMemory => .hyperlink_bytes, + error.NeedsRehash => null, + }); + + // We need to recreate the link into the new page. + const dst_link2 = src_link.dupe( + src_page, + self.page, + ) catch |err2| { + // This shouldn't fail since we did a capacity + // check above. + log.err("link dupe failed with capacity check err={}", .{err2}); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + cell.hyperlink = false; + break :hyperlink; + }; + + // We assume this one will succeed. We dupe the link + // again, and don't have to worry about the other one + // because increasing the capacity naturally clears up + // any managed memory not associated with a cell yet. + break :id self.page.hyperlink_set.addWithIdContext( + self.page.memory, + dst_link2, + src_id, + .{ .page = self.page }, + ) catch |err2| { + // This shouldn't happen since we increased capacity + // above so we handle it like the other similar + // cases and log it, crash in safe builds, and + // remove the hyperlink in unsafe builds. + log.err( + "addWithIdContext failed after capacity increase err={}", + .{err2}, + ); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + dst_link2.free(self.page); + cell.hyperlink = false; + break :hyperlink; + }; + } orelse src_id; + + // We expect this to succeed due to the hyperlinkCapacity + // check we did before. If it doesn't succeed let's + // log it, crash (in safe builds), and clear our state. + self.page.setHyperlink( + self.page_row, + self.page_cell, + dst_id, + ) catch |err| { + log.err( + "setHyperlink failed after capacity increase err={}", + .{err}, + ); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + // Unsafe builds we throw away hyperlink data! + self.page.hyperlink_set.release(self.page.memory, dst_id); + cell.hyperlink = false; + break :hyperlink; + }; + } + + // Copy style data. + if (cell.hasStyling()) style: { + const style = src_page.styles.get( + src_page.memory, + cell.style_id, + ).*; + + const id = self.page.styles.addWithId( + self.page.memory, + style, + cell.style_id, + ) catch |err| id: { + // If the add failed then either the set needs to grow + // or it needs to be rehashed. Either one of those can + // be accomplished by increasing capacity, either with + // no actual change or with an increased style cap. + try self.increaseCapacity(list, switch (err) { + error.OutOfMemory => .styles, + error.NeedsRehash => null, + }); + + // We assume this one will succeed. + break :id self.page.styles.addWithId( + self.page.memory, + style, + cell.style_id, + ) catch |err2| { + // Should not fail since we just modified capacity + // above. Log it, crash in safe builds, clear style + // in unsafe builds. + log.err( + "addWithId failed after capacity increase err={}", + .{err2}, + ); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + cell.style_id = stylepkg.default_id; + break :style; + }; + } orelse cell.style_id; + + self.page_row.styled = true; + self.page_cell.style_id = id; + } + + self.cursorForward(); + return .success; + } + /// Create a new page in the provided list with the provided /// capacity then clone the row currently being worked on to /// it and delete it from the old page. Places cursor in the @@ -1654,7 +1689,7 @@ const ReflowCursor = struct { self: *ReflowCursor, list: *PageList, adjustment: ?IncreaseCapacity, - ) !void { + ) IncreaseCapacityError!void { const old_x = self.x; const old_y = self.y; const old_total_rows = self.total_rows; From 97621ece935228efe82bfb451d3be75051e12cf1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Jan 2026 07:21:17 -0800 Subject: [PATCH 12/18] terminal: handle reflowRow OutOfSpace by no-op --- src/terminal/PageList.zig | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 0ca00f45c..2045ccc01 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -874,7 +874,7 @@ pub const Resize = struct { /// Resize /// TODO: docs -pub fn resize(self: *PageList, opts: Resize) !void { +pub fn resize(self: *PageList, opts: Resize) Allocator.Error!void { defer self.assertIntegrity(); if (comptime std.debug.runtime_safety) { @@ -944,7 +944,7 @@ fn resizeCols( self: *PageList, cols: size.CellCountInt, cursor: ?Resize.Cursor, -) !void { +) Allocator.Error!void { assert(cols != self.cols); // Update our cols. We have to do this early because grow() that we @@ -1149,7 +1149,7 @@ const ReflowCursor = struct { self: *ReflowCursor, list: *PageList, row: Pin, - ) !void { + ) Allocator.Error!void { const src_page: *Page = &row.node.data; const src_row = row.rowAndCell().row; const src_y = row.y; @@ -1247,11 +1247,11 @@ const ReflowCursor = struct { } } - switch (try self.writeCell( + if (self.writeCell( list, &cells[x], src_page, - )) { + )) |result| switch (result) { // Wrote the cell, move to the next. .success => x += 1, // Wrote the cell but request to skip the next so skip it. @@ -1259,6 +1259,24 @@ const ReflowCursor = struct { .skip_next => x += 2, // Didn't write the cell, repeat writing this same cell. .repeat => {}, + } else |err| switch (err) { + // System out of memory, we can't fix this. + error.OutOfMemory => return error.OutOfMemory, + + // We reached the capacity of a single page and can't + // add any more of some type of managed memory. + error.OutOfSpace => { + log.warn("OutOfSpace during reflow at src_y={} src_x={} dst_y={} dst_x={} cp={X}", .{ + src_y, + x, + self.y, + self.x, + cells[x].content.codepoint, + }); + + // TODO: Split the page + x += 1; + }, } } From 42321cc7d59f3c3f02c14e225c0d374c8a002339 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Jan 2026 07:24:37 -0800 Subject: [PATCH 13/18] terminal: write to the proper cell --- src/terminal/PageList.zig | 160 ++++++++++++++++++++------------------ 1 file changed, 85 insertions(+), 75 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 2045ccc01..84518e8b7 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -1308,84 +1308,98 @@ const ReflowCursor = struct { repeat, skip_next, } { - // Copy cell contents. - switch (cell.content_tag) { - .codepoint, - .codepoint_grapheme, - => switch (cell.wide) { - .narrow => self.page_cell.* = cell.*, + // Initialize self.page_cell with basic, unmanaged memory contents. + { + // This must not fail because we want to make sure we atomically + // setup our page cell to be valid. + errdefer comptime unreachable; - .wide => if (self.page.size.cols > 1) { - if (self.x == self.page.size.cols - 1) { - // If there's a wide character in the last column of - // the reflowed page then we need to insert a spacer - // head and wrap before handling it. - self.page_cell.* = .{ - .content_tag = .codepoint, - .content = .{ .codepoint = 0 }, - .wide = .spacer_head, - }; + // Copy cell contents. + switch (cell.content_tag) { + .codepoint, + .codepoint_grapheme, + => switch (cell.wide) { + .narrow => self.page_cell.* = cell.*, - // Move to the next row (this sets pending wrap - // which will cause us to wrap on the next - // iteration). + .wide => if (self.page.size.cols > 1) { + if (self.x == self.page.size.cols - 1) { + // If there's a wide character in the last column of + // the reflowed page then we need to insert a spacer + // head and wrap before handling it. + self.page_cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = 0 }, + .wide = .spacer_head, + }; + + // Move to the next row (this sets pending wrap + // which will cause us to wrap on the next + // iteration). + self.cursorForward(); + + // Decrement the source position so that when we + // loop we'll process this source cell again, + // since we can't copy it into a spacer head. + return .repeat; + } else { + self.page_cell.* = cell.*; + } + } else { + // Edge case, when resizing to 1 column, wide + // characters are just destroyed and replaced + // with empty narrow cells. + self.page_cell.content.codepoint = 0; + self.page_cell.wide = .narrow; self.cursorForward(); - // Decrement the source position so that when we - // loop we'll process this source cell again, - // since we can't copy it into a spacer head. - return .repeat; - } else { + // Skip spacer tail so it doesn't cause a wrap. + return .skip_next; + }, + + .spacer_tail => if (self.page.size.cols > 1) { self.page_cell.* = cell.*; - } - } else { - // Edge case, when resizing to 1 column, wide - // characters are just destroyed and replaced - // with empty narrow cells. - self.page_cell.content.codepoint = 0; - self.page_cell.wide = .narrow; - self.cursorForward(); + } else { + // Edge case, when resizing to 1 column, wide + // characters are just destroyed and replaced + // with empty narrow cells, so we should just + // discard any spacer tails. + return .success; + }, - // Skip spacer tail so it doesn't cause a wrap. - return .skip_next; + .spacer_head => { + // Spacer heads should be ignored. If we need a + // spacer head in our reflowed page, it is added + // when processing the wide cell it belongs to. + return .success; + }, }, - .spacer_tail => if (self.page.size.cols > 1) { + .bg_color_palette, + .bg_color_rgb, + => { + // These are guaranteed to have no style or grapheme + // data associated with them so we can fast path them. self.page_cell.* = cell.*; - } else { - // Edge case, when resizing to 1 column, wide - // characters are just destroyed and replaced - // with empty narrow cells, so we should just - // discard any spacer tails. + self.cursorForward(); return .success; }, + } - .spacer_head => { - // Spacer heads should be ignored. If we need a - // spacer head in our reflowed page, it is added - // when processing the wide cell it belongs to. - return .success; - }, - }, + // These will create issues by trying to clone managed memory that + // isn't set if the current dst row needs to be moved to a new page. + // They'll be fixed once we do properly copy the relevant memory. + self.page_cell.content_tag = .codepoint; + self.page_cell.hyperlink = false; + self.page_cell.style_id = stylepkg.default_id; - .bg_color_palette, - .bg_color_rgb, - => { - // These are guaranteed to have no style or grapheme - // data associated with them so we can fast path them. - self.page_cell.* = cell.*; - self.cursorForward(); - return .success; - }, + if (comptime build_options.kitty_graphics) { + // Copy Kitty virtual placeholder status + if (cell.codepoint() == kitty.graphics.unicode.placeholder) { + self.page_row.kitty_virtual_placeholder = true; + } + } } - // These will create issues by trying to clone managed memory that - // isn't set if the current dst row needs to be moved to a new page. - // They'll be fixed once we do properly copy the relevant memory. - self.page_cell.content_tag = .codepoint; - self.page_cell.hyperlink = false; - self.page_cell.style_id = stylepkg.default_id; - // std.log.warn("\nsrc_y={} src_x={} dst_y={} dst_x={} dst_cols={} cp={X} wide={} page_cell_wide={}", .{ // src_y, // x, @@ -1397,12 +1411,10 @@ const ReflowCursor = struct { // self.page_cell.wide, // }); - if (comptime build_options.kitty_graphics) { - // Copy Kitty virtual placeholder status - if (cell.codepoint() == kitty.graphics.unicode.placeholder) { - self.page_row.kitty_virtual_placeholder = true; - } - } + // From this point on we're moving on to failable, managed memory. + // If we reach an error, we do the minimal cleanup necessary to + // not leave dangling memory but otherwise we gracefully degrade + // into some functional but not strictly correct cell. // Copy grapheme data. if (cell.content_tag == .codepoint_grapheme) { @@ -1455,8 +1467,8 @@ const ReflowCursor = struct { } // Unsafe builds we throw away grapheme data! - cell.content_tag = .codepoint; - cell.content = .{ .codepoint = 0xFFFD }; + self.page_cell.content_tag = .codepoint; + self.page_cell.content = .{ .codepoint = 0xFFFD }; }; } @@ -1548,7 +1560,6 @@ const ReflowCursor = struct { unreachable; } - cell.hyperlink = false; break :hyperlink; }; @@ -1576,7 +1587,6 @@ const ReflowCursor = struct { } dst_link2.free(self.page); - cell.hyperlink = false; break :hyperlink; }; } orelse src_id; @@ -1600,7 +1610,7 @@ const ReflowCursor = struct { // Unsafe builds we throw away hyperlink data! self.page.hyperlink_set.release(self.page.memory, dst_id); - cell.hyperlink = false; + self.page_cell.hyperlink = false; break :hyperlink; }; } @@ -1644,7 +1654,7 @@ const ReflowCursor = struct { unreachable; } - cell.style_id = stylepkg.default_id; + self.page_cell.style_id = stylepkg.default_id; break :style; }; } orelse cell.style_id; From c9d15949d85564aa47763de2da341ae6654f6e90 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Jan 2026 08:22:53 -0800 Subject: [PATCH 14/18] terminal: on reflow OutOfSpace, move last row to new page and try again --- src/terminal/PageList.zig | 60 +++++++++++++++++++++++++++++---------- 1 file changed, 45 insertions(+), 15 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 84518e8b7..710c0aaa7 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -1264,18 +1264,22 @@ const ReflowCursor = struct { error.OutOfMemory => return error.OutOfMemory, // We reached the capacity of a single page and can't - // add any more of some type of managed memory. - error.OutOfSpace => { - log.warn("OutOfSpace during reflow at src_y={} src_x={} dst_y={} dst_x={} cp={X}", .{ - src_y, - x, - self.y, - self.x, - cells[x].content.codepoint, - }); - - // TODO: Split the page + // add any more of some type of managed memory. When this + // happens we split out the current row we're working on + // into a new page and continue from there. + error.OutOfSpace => if (self.y == 0) { + // If we're already on the first-row, we can't split + // any further, so we just ignore bad cells and take + // corrupted (but valid) cell contents. + log.warn("reflowRow OutOfSpace on first row, discarding cell managed memory", .{}); x += 1; + self.cursorForward(); + } else { + // Move our last row to a new page. + try self.moveLastRowToNewPage(list, cap); + + // Do NOT increment x so that we retry writing + // the same existing cell. }, } } @@ -1683,7 +1687,7 @@ const ReflowCursor = struct { self: *ReflowCursor, list: *PageList, cap: Capacity, - ) !void { + ) Allocator.Error!void { assert(self.y == self.page.size.rows - 1); assert(!self.pending_wrap); @@ -1693,15 +1697,41 @@ const ReflowCursor = struct { const old_x = self.x; try self.cursorNewPage(list, cap); + assert(self.node != old_node); + assert(self.y == 0); // Restore the x position of the cursor. self.cursorAbsolute(old_x, 0); - // We expect to have enough capacity to clone the row. - try self.page.cloneRowFrom(old_page, self.page_row, old_row); + // Copy our old data. This should NOT fail because we have the + // capacity of the old page which already fits the data we requested. + self.page.cloneRowFrom( + old_page, + self.page_row, + old_row, + ) catch |err| { + log.err( + "error cloning single row for moveLastRowToNewPage err={}", + .{err}, + ); + @panic("unexpected copy row failure"); + }; + + // Move any tracked pins from that last row into this new node. + { + const pin_keys = list.tracked_pins.keys(); + for (pin_keys) |p| { + if (&p.node.data != old_page or + p.y != old_page.size.rows - 1) continue; + + p.node = self.node; + p.y = self.y; + // p.x remains the same since we're copying the row as-is + } + } // Clear the row from the old page and truncate it. - old_page.clearCells(old_row, 0, self.page.size.cols); + old_page.clearCells(old_row, 0, old_page.size.cols); old_page.size.rows -= 1; // If that was the last row in that page From ec0a1500984403eefffbbf7528cf22230b08cfb1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Jan 2026 09:12:41 -0800 Subject: [PATCH 15/18] terminal: moveLastRowToNewPage needs to fix up total_rows --- src/terminal/PageList.zig | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 710c0aaa7..69edcafb1 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -1696,10 +1696,18 @@ const ReflowCursor = struct { const old_row = self.page_row; const old_x = self.x; + // Our total row count never changes, because we're removing one + // row from the last page and moving it into a new page. + const old_total_rows = self.total_rows; + defer self.total_rows = old_total_rows; + try self.cursorNewPage(list, cap); assert(self.node != old_node); assert(self.y == 0); + // We have no cleanup for our old state from here on out. No failures! + errdefer comptime unreachable; + // Restore the x position of the cursor. self.cursorAbsolute(old_x, 0); @@ -1752,7 +1760,7 @@ const ReflowCursor = struct { const old_y = self.y; const old_total_rows = self.total_rows; - self.* = .init(node: { + const node = node: { // Pause integrity checks because the total row count won't // be correct during a reflow. list.pauseIntegrityChecks(true); @@ -1761,8 +1769,12 @@ const ReflowCursor = struct { self.node, adjustment, ); - }); + }; + // We must not fail after this, we've modified our self.node + // and we need to fix it up. + errdefer comptime unreachable; + self.* = .init(node); self.cursorAbsolute(old_x, old_y); self.total_rows = old_total_rows; } @@ -1824,7 +1836,6 @@ const ReflowCursor = struct { list.pages.insertAfter(self.node, node); self.* = .init(node); - self.new_rows = new_rows; } From 85a3d623b2f3452a49eb634c10e46ba6746d0cd6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Jan 2026 13:42:34 -0800 Subject: [PATCH 16/18] terminal: increaseCapacity should reach maxInt before overflow --- src/terminal/PageList.zig | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 69edcafb1..b027f54d4 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -3003,8 +3003,16 @@ pub fn increaseCapacity( // 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| { + 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; }; @field(cap, field_name) = new; @@ -6844,18 +6852,19 @@ test "PageList increaseCapacity returns OutOfSpace at max capacity" { var s = try init(alloc, 2, 2, 0); defer s.deinit(); - // Keep increasing styles capacity until we're at more than half of max + // Keep increasing styles capacity until we get OutOfSpace const max_styles = std.math.maxInt(size.StyleCountInt); - const half_max = max_styles / 2 + 1; - while (s.pages.first.?.data.capacity.styles < half_max) { - _ = try s.increaseCapacity(s.pages.first.?, .styles); + while (true) { + _ = s.increaseCapacity( + s.pages.first.?, + .styles, + ) catch |err| { + // Before OutOfSpace, we should have reached maxInt + try testing.expectEqual(error.OutOfSpace, err); + try testing.expectEqual(max_styles, s.pages.first.?.data.capacity.styles); + break; + }; } - - // Now increaseCapacity should fail with OutOfSpace - try testing.expectError( - error.OutOfSpace, - s.increaseCapacity(s.pages.first.?, .styles), - ); } test "PageList increaseCapacity after col shrink" { From 442a395850f30a2b6fc455414359368dfbefb9fc Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Jan 2026 14:24:02 -0800 Subject: [PATCH 17/18] terminal: ensure our std_capacity fits within max page size --- src/terminal/PageList.zig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index b027f54d4..d8354916b 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -292,7 +292,8 @@ fn initialCapacity(cols: size.CellCountInt) Capacity { comptime { var cap = std_capacity; cap.cols = std.math.maxInt(size.CellCountInt); - _ = Page.layout(cap); + const layout = Page.layout(cap); + assert(layout.total_size <= size.max_page_size); } if (std_capacity.adjust( From df35363b15ef1c1284ab0ad3615e43fec4777316 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Jan 2026 14:35:14 -0800 Subject: [PATCH 18/18] terminal: more robust handling of max_page_size not 100% --- src/terminal/PageList.zig | 19 ++++++++++++++++++- src/terminal/page.zig | 2 ++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index d8354916b..3e6a39a3f 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -414,6 +414,10 @@ fn initPages( const pooled = layout.total_size <= std_size; const page_alloc = pool.pages.arena.child_allocator; + // Guaranteed by comptime checks in initialCapacity but + // redundant here for safety. + assert(layout.total_size <= size.max_page_size); + var rem = rows; while (rem > 0) { const node = try pool.nodes.create(); @@ -2091,7 +2095,7 @@ fn resizeWithoutReflowGrowCols( ) catch |err| err: { comptime assert(@TypeOf(err) == error{OutOfMemory}); - // We verify all maxed out page layouts work. + // We verify all maxed out page layouts don't overflow, var cap = page.capacity; cap.cols = cols; @@ -3017,6 +3021,14 @@ pub fn increaseCapacity( return error.OutOfSpace; }; @field(cap, field_name) = new; + + // If our capacity exceeds the maximum page size, treat it + // as an OutOfSpace because things like page splitting will + // help. + const layout = Page.layout(cap); + if (layout.total_size > size.max_page_size) { + return error.OutOfSpace; + } }, }; @@ -3091,6 +3103,11 @@ inline fn createPageExt( const pooled = layout.total_size <= std_size; const page_alloc = pool.pages.arena.child_allocator; + // It would be better to encode this into the Zig error handling + // system but that is a big undertaking and we only have a few + // centralized call sites so it is handled on its own currently. + assert(layout.total_size <= size.max_page_size); + // Our page buffer comes from our standard memory pool if it // is within our standard size since this is what the pool // dispenses. Otherwise, we use the heap allocator to allocate. diff --git a/src/terminal/page.zig b/src/terminal/page.zig index 1150027a4..075f03b57 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -2046,6 +2046,8 @@ test "Page.layout can take a maxed capacity" { @field(cap, field.name) = std.math.maxInt(field.type); } + // Note that a max capacity will exceed our max_page_size so we + // can't init a page with it, but it should layout. _ = Page.layout(cap); }