diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index fc680b971..3e6a39a3f 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( @@ -413,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(); @@ -874,7 +879,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 +949,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 @@ -992,32 +997,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. @@ -1110,20 +1154,15 @@ 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; - - // 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 +1184,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 +1202,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); @@ -1199,8 +1252,76 @@ const ReflowCursor = struct { } } - const cell = &cells[x]; - x += 1; + 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. + // This is used for things like spacers. + .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. 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. + }, + } + } + + // If the source row isn't wrapped then we should scroll afterwards. + if (!src_row.wrap) { + self.new_rows += 1; + } + } + + /// 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, + } { + // 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; // Copy cell contents. switch (cell.content_tag) { @@ -1220,16 +1341,15 @@ const ReflowCursor = struct { .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; + + // 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.*; } @@ -1240,9 +1360,9 @@ const ReflowCursor = struct { 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; + return .skip_next; }, .spacer_tail => if (self.page.size.cols > 1) { @@ -1252,14 +1372,14 @@ const ReflowCursor = struct { // characters are just destroyed and replaced // with empty narrow cells, so we should just // discard any spacer tails. - continue; + 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. - continue; + return .success; }, }, @@ -1270,7 +1390,7 @@ const ReflowCursor = struct { // data associated with them so we can fast path them. self.page_cell.* = cell.*; self.cursorForward(); - continue; + return .success; }, } @@ -1281,185 +1401,279 @@ const ReflowCursor = struct { 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.adjustCapacity(list, .{ - .grapheme_bytes = cap.grapheme_bytes * 2, - }); - } - - // 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: usize = cap.grapheme_bytes; - while (new_grapheme_capacity - cap.grapheme_bytes < required) { - new_grapheme_capacity *= 2; - } - try self.adjustCapacity(list, .{ - .grapheme_bytes = new_grapheme_capacity, - }); - } - - // This shouldn't fail since we made sure we have space above. - try self.page.setGraphemes(self.page_row, self.page_cell, cps); - } - - // Copy hyperlink data. - if (cell.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.adjustCapacity(list, .{ - .hyperlink_bytes = cap.hyperlink_bytes * 2, - }); - } - - // 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, - }; - 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: usize = cap.string_bytes; - while (new_string_capacity - cap.string_bytes < additional_required_string_capacity) { - new_string_capacity *= 2; - } - try self.adjustCapacity(list, .{ - .string_bytes = new_string_capacity, - }); - } - - 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), - 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 adjusting 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 => .{}, - }); - - // 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 - // any managed memory not associated with a cell yet. - break :id try self.page.hyperlink_set.addWithIdContext( - self.page.memory, - try src_link.dupe(src_page, self.page), - src_id, - .{ .page = self.page }, - ); - } orelse src_id; - - // We expect this to succeed due to the - // hyperlinkCapacity check we did before. - try self.page.setHyperlink( - self.page_row, - self.page_cell, - dst_id, - ); - } - - // Copy style data. - if (cell.hasStyling()) { - 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 adjusting 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 => .{}, - }); - - // We assume this one will succeed. - break :id try self.page.styles.addWithId( - self.page.memory, - style, - cell.style_id, - ); - } 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. - if (!src_row.wrap) { - self.new_rows += 1; + // 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, + // }); + + // 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) { + // 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! + self.page_cell.content_tag = .codepoint; + self.page_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; + } + + 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); + 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); + self.page_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; + } + + self.page_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 @@ -1478,7 +1692,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); @@ -1487,16 +1701,50 @@ 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); - // 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 @@ -1507,27 +1755,31 @@ 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, - ) !void { + adjustment: ?IncreaseCapacity, + ) IncreaseCapacityError!void { const old_x = self.x; 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); defer list.pauseIntegrityChecks(false); - break :node try list.adjustCapacity( + break :node try list.increaseCapacity( 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; } @@ -1578,17 +1830,17 @@ 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); self.* = .init(node); - self.new_rows = new_rows; } @@ -1598,7 +1850,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. @@ -1656,7 +1908,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; @@ -1816,26 +2068,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 don't overflow, + 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 { @@ -1899,6 +2169,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) { @@ -1911,19 +2206,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); @@ -1940,6 +2249,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); @@ -2518,7 +2831,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.?; @@ -2537,7 +2850,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 @@ -2642,75 +2955,82 @@ 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: ?usize = null, - - /// Adjust the number of available grapheme bytes in the page. - grapheme_bytes: ?usize = null, - - /// Adjust the number of available hyperlink bytes in the page. - hyperlink_bytes: ?usize = null, - - /// Adjust the number of available string bytes in the page. - string_bytes: ?usize = null, +/// Possible dimensions to increase capacity for. +pub const IncreaseCapacity = enum { + styles, + grapheme_bytes, + hyperlink_bytes, + string_bytes, }; -pub const AdjustCapacityError = Allocator.Error || Page.CloneFromError; +pub const IncreaseCapacityError = error{ + // An actual system OOM trying to allocate memory. + OutOfMemory, -/// 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. + // 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. /// -/// 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. +/// 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. /// -/// 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( +/// 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: AdjustCapacity, -) AdjustCapacityError!*List.Node { + adjustment: ?IncreaseCapacity, +) IncreaseCapacityError!*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. + // 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); - // 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; - 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; - 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; - 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; - cap.string_bytes = @max(cap.string_bytes, aligned); - } + // We use checked math to prevent overflow. If there is an + // overflow it means we're out of space in this dimension, + // since pages can take up to their maxInt capacity in any + // category. + const new = std.math.mul( + Int, + old, + 2, + ) catch |err| overflow: { + comptime assert(@TypeOf(err) == error{Overflow}); + // Our final doubling would overflow since maxInt is + // 2^N - 1 for an unsignged int of N bits. So, if we overflow + // and we haven't used all the bits, use all the bits. + if (old < std.math.maxInt(Int)) break :overflow std.math.maxInt(Int); + return error.OutOfSpace; + }; + @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; + } + }, + }; log.info("adjusting page capacity={}", .{cap}); @@ -2722,7 +3042,22 @@ pub fn adjustCapacity( 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); + 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(); @@ -2768,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. @@ -4891,21 +5231,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); } @@ -6324,12 +6657,15 @@ test "PageList eraseRowBounded exhausts pages invalidates viewport offset cache" }, s.scrollbar()); } -test "PageList adjustCapacity to increase styles" { +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; @@ -6347,14 +6683,19 @@ test "PageList adjustCapacity to increase styles" { } // Increase our styles - _ = try s.adjustCapacity( - s.pages.first.?, - .{ .styles = std_capacity.styles * 2 }, - ); + _ = 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); @@ -6367,17 +6708,19 @@ test "PageList adjustCapacity to increase styles" { } } -test "PageList adjustCapacity to increase graphemes" { +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; - // 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); @@ -6389,15 +6732,14 @@ test "PageList adjustCapacity to increase graphemes" { } } - // Increase our graphemes - _ = try s.adjustCapacity( - s.pages.first.?, - .{ .grapheme_bytes = std_capacity.grapheme_bytes * 2 }, - ); + _ = 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); @@ -6410,17 +6752,19 @@ test "PageList adjustCapacity to increase graphemes" { } } -test "PageList adjustCapacity to increase hyperlinks" { +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; - // 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); @@ -6432,15 +6776,14 @@ test "PageList adjustCapacity to increase hyperlinks" { } } - // Increase our graphemes - _ = try s.adjustCapacity( - s.pages.first.?, - .{ .hyperlink_bytes = @max(std_capacity.hyperlink_bytes * 2, 2048) }, - ); + _ = 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); @@ -6453,39 +6796,162 @@ test "PageList adjustCapacity to increase hyperlinks" { } } -test "PageList adjustCapacity after col shrink" { +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(); + + // Keep increasing styles capacity until we get OutOfSpace + const max_styles = std.math.maxInt(size.StyleCountInt); + 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; + }; + } +} + +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 - this updates size.cols but not capacity.cols + // Shrink columns 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 }, - ); + // Increase capacity + _ = try s.increaseCapacity(s.pages.first.?, .styles); { const page = &s.pages.first.?.data; - // After adjustCapacity, size.cols should still be 5, not 10 + // 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; @@ -11090,12 +11556,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); diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index fed0a8c51..1538da5da 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -517,39 +517,38 @@ 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( +pub fn increaseCapacity( self: *Screen, node: *PageList.List.Node, - adjustment: PageList.AdjustCapacity, -) PageList.AdjustCapacityError!*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. - if (node != self.cursor.page_pin.node) { - return try self.pages.adjustCapacity(node, adjustment); - } + // 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 adjust the + // 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.adjustCapacity(node, adjustment); + 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 != 0) { + 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.adjustCapacity) Failed to add cursor style back to page, err={}", + "(Screen.increaseCapacity) Failed to add cursor style back to page, err={}", .{err}, ); @@ -571,7 +570,7 @@ pub fn adjustCapacity( 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={}", + "(Screen.increaseCapacity) Failed to add cursor hyperlink back to page, err={}", .{err}, ); }; @@ -1106,14 +1105,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. @@ -1930,7 +1934,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()}); @@ -1949,6 +1963,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. @@ -1960,30 +1977,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, @@ -2003,11 +2040,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 @@ -2018,17 +2053,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). @@ -2036,7 +2076,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, @@ -2061,21 +2101,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, ), } @@ -2137,7 +2177,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; @@ -2152,40 +2192,38 @@ 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 + // 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. // - // 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 @@ -3007,15 +3045,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 => { @@ -8896,134 +8934,240 @@ 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" { +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 }); + var s = try init(alloc, .{ + .cols = 5, + .rows = 5, + .max_scrollback = 0, + }); defer s.deinit(); - - try s.setAttribute(.{ .bold = {} }); + 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, // All chars + cursor + 6, page.styles.refCount(page.memory, s.cursor.style_id), ); } - // This forces the page to change. - _ = try s.adjustCapacity( + // This forces the page to change via increaseCapacity. + const new_node = try s.increaseCapacity( s.cursor.page_pin.node, - .{ .grapheme_bytes = s.cursor.page_pin.node.data.capacity.grapheme_bytes * 2 }, + .grapheme_bytes, ); - // Our ref counts should still be the same + // 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, // All chars + cursor + 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: adjustCapacity cursor hyperlink exceeds string alloc size" { +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 = 0 }); + var s = try init(alloc, .{ + .cols = 80, + .rows = 24, + .max_scrollback = 10000, + }); 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. + // 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"); - // 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, .{}); + // 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 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); -} + // 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.?); -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"); - } + // 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(); - // 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, .{}); + // Now we have two pages + try testing.expect(s.pages.pages.first != s.pages.pages.last); + const second_page = s.pages.pages.last.?; - // 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); + // 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); } test "Screen: cursorDown to page with insufficient capacity" { diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index d717a9724..3740397d3 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 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 + // 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. 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..075f03b57 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,21 @@ 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 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; + inline for (@typeInfo(Capacity).@"struct".fields) |field| { + @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); +} + 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;