Applied-Software/ghostty
5
0
Fork 0
mirror of https://github.com/ghostty-org/ghostty.git synced 2025-07-17 01:06:08 +03:00
Code Issues Packages Projects Releases Wiki Activity

terminal/page: discriminate cloneFrom errors

Browse Source 
Doing this makes it possible to appropriately handle a failed cloneFrom
operation by adjusting the correct part of the page capacity accordingly
This commit is contained in:
Qwerasd
2024-08-30 13:22:50 -04:00
parent 3bc2dbfa72
commit daa1755793
2 changed files with 110 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/terminal/Screen.zig
View File

@ -433,7 +433,7 @@ pub fn clonePool(
/// Adjust the capacity of a page within the pagelist of this screen. /// Adjust the capacity of a page within the pagelist of this screen.
/// This handles some accounting if the page being modified is the /// This handles some accounting if the page being modified is the
/// cursor page. /// cursor page.
fn adjustCapacity( pub fn adjustCapacity(
self: *Screen, self: *Screen,
page: *PageList.List.Node, page: *PageList.List.Node,
adjustment: PageList.AdjustCapacity, adjustment: PageList.AdjustCapacity,
@ -1792,7 +1792,7 @@ pub fn cursorSetHyperlink(self: *Screen) !void {
return; return;
} else |err| switch (err) { } else |err| switch (err) {
// hyperlink_map is out of space, realloc the page to be larger // hyperlink_map is out of space, realloc the page to be larger
error.OutOfMemory => { error.HyperlinkMapOutOfMemory => {
_ = try self.adjustCapacity( _ = try self.adjustCapacity(
self.cursor.page_pin.page, self.cursor.page_pin.page,
.{ .hyperlink_bytes = page.capacity.hyperlink_bytes * 2 }, .{ .hyperlink_bytes = page.capacity.hyperlink_bytes * 2 },

164
src/terminal/page.zig
View File

@ -611,7 +611,27 @@ pub const Page = struct {
return result; return result;
} }
pub const CloneFromError = Allocator.Error || style.Set.AddError; pub const StyleSetError = error{
StyleSetOutOfMemory,
StyleSetNeedsRehash,
};
pub const HyperlinkError = error{
StringAllocOutOfMemory,
HyperlinkSetOutOfMemory,
HyperlinkSetNeedsRehash,
HyperlinkMapOutOfMemory,
};
pub const GraphemeError = error{
GraphemeMapOutOfMemory,
GraphemeAllocOutOfMemory,
};
pub const CloneFromError =
StyleSetError ||
HyperlinkError ||
GraphemeError;
/// Clone the contents of another page into this page. The capacities /// Clone the contents of another page into this page. The capacities
/// can be different, but the size of the other page must fit into /// can be different, but the size of the other page must fit into
@ -731,6 +751,16 @@ pub const Page = struct {
// get all of that right. // get all of that right.
for (cells, other_cells) |*dst_cell, *src_cell| { for (cells, other_cells) |*dst_cell, *src_cell| {
dst_cell.* = src_cell.*; dst_cell.* = src_cell.*;
// Reset any managed memory markers on the cell so that we don't
// hit an integrity check if we have to return an error because
// the page can't fit the new memory.
dst_cell.hyperlink = false;
dst_cell.style_id = style.default_id;
if (dst_cell.content_tag == .codepoint_grapheme) {
dst_cell.content_tag = .codepoint;
}
if (src_cell.hasGrapheme()) { if (src_cell.hasGrapheme()) {
// To prevent integrity checks flipping. This will // To prevent integrity checks flipping. This will
// get fixed up when we check the style id below. // get fixed up when we check the style id below.
@ -738,13 +768,12 @@ pub const Page = struct {
dst_cell.style_id = style.default_id; dst_cell.style_id = style.default_id;
} }
dst_cell.content_tag = .codepoint; // required for appendGrapheme // Copy the grapheme codepoints
const cps = other.lookupGrapheme(src_cell).?; const cps = other.lookupGrapheme(src_cell).?;
for (cps) |cp| try self.appendGrapheme(dst_row, dst_cell, cp);
try self.setGraphemes(dst_row, dst_cell, cps);
} }
if (src_cell.hyperlink) hyperlink: { if (src_cell.hyperlink) hyperlink: {
dst_row.hyperlink = true;
const id = other.lookupHyperlink(src_cell).?; const id = other.lookupHyperlink(src_cell).?;
// Fast-path: same page we can add with the same id. // Fast-path: same page we can add with the same id.
@ -757,55 +786,60 @@ pub const Page = struct {
// Slow-path: get the hyperlink from the other page, // Slow-path: get the hyperlink from the other page,
// add it, and migrate. // add it, and migrate.
const dst_link = dst_link: { // If our page can't support an additional cell with
// Fast path is we just dupe the hyperlink because // a hyperlink then we have to return an error.
// it doesn't require traversal through the hyperlink if (self.hyperlinkCount() >= self.hyperlinkCapacity() - 1) {
// map. If the add below already contains it then it'll // The hyperlink map capacity needs to be increased.
// call the deleted context callback and we'll free return error.HyperlinkMapOutOfMemory;
// this back. }
const other_link = other.hyperlink_set.get(other.memory, id);
if (other_link.dupe(other, self)) |dst_link| { const other_link = other.hyperlink_set.get(other.memory, id);
break :dst_link dst_link; const dst_id = dst_id: {
} else |err| switch (err) { // First check if the link already exists in our page,
// If this happens, the only possible valid outcome is // and increment its refcount if so, since we're about
// that it is because this link is already in our set // to use it.
// and our memory is full because we already have it. if (self.hyperlink_set.lookupContext(
// Any other outcome is an integrity violation. self.memory,
error.OutOfMemory => {}, other_link.*,
.{ .page = @constCast(other) },
// `lookupContext` uses the context for hashing, and
// that doesn't write to the page, so this constCast
// is completely safe.
)) |i| {
self.hyperlink_set.use(self.memory, i);
break :dst_id i;
} }
// Slow, the only way to really find our link is to // If we don't have this link in our page yet then
// traverse over the map, which includes dupes... // we need to clone it over and add it to our set.
const dst_map = self.hyperlink_map.map(self.memory);
var it = dst_map.valueIterator();
while (it.next()) |existing_id| {
const existing_link = self.hyperlink_set.get(
self.memory,
existing_id.*,
);
if (existing_link.eql( // Clone the link.
self.memory, const dst_link = other_link.dupe(other, self) catch |e| {
other_link, comptime assert(@TypeOf(e) == error{OutOfMemory});
other.memory, // The string alloc capacity needs to be increased.
)) { return error.StringAllocOutOfMemory;
break :dst_link existing_link.*; };
}
}
// There is no other valid scenario where we don't // Add it, preferring to use the same ID as the other
// have the memory to dupe a hyperlink since we allocate // page, since this *probably* speeds up full-page
// cloned pages with enough capacity to contain their // clones.
// contents. //
unreachable; // TODO(qwerasd): verify the assumption that `addWithId`
// is ever actually useful, I think it may not be.
break :dst_id self.hyperlink_set.addWithIdContext(
self.memory,
dst_link,
id,
.{ .page = self },
) catch |e| switch (e) {
// The hyperlink set capacity needs to be increased.
error.OutOfMemory => return error.HyperlinkSetOutOfMemory,
// The hyperlink set needs to be rehashed.
error.NeedsRehash => return error.HyperlinkSetNeedsRehash,
} orelse id;
}; };
const dst_id = try self.hyperlink_set.addWithIdContext(
self.memory,
dst_link,
id,
.{ .page = self },
) orelse id;
try self.setHyperlink(dst_row, dst_cell, dst_id); try self.setHyperlink(dst_row, dst_cell, dst_id);
} }
if (src_cell.style_id != style.default_id) style: { if (src_cell.style_id != style.default_id) style: {
@ -822,11 +856,17 @@ pub const Page = struct {
// Slow path: Get the style from the other // Slow path: Get the style from the other
// page and add it to this page's style set. // page and add it to this page's style set.
const other_style = other.styles.get(other.memory, src_cell.style_id); const other_style = other.styles.get(other.memory, src_cell.style_id);
dst_cell.style_id = try self.styles.addWithId( dst_cell.style_id = self.styles.addWithId(
self.memory, self.memory,
other_style.*, other_style.*,
src_cell.style_id, src_cell.style_id,
) orelse src_cell.style_id; ) catch |e| switch (e) {
// The style set capacity needs to be increased.
error.OutOfMemory => return error.StyleSetOutOfMemory,
// The style set needs to be rehashed.
error.NeedsRehash => return error.StyleSetNeedsRehash,
} orelse src_cell.style_id;
} }
if (src_cell.codepoint() == kitty.graphics.unicode.placeholder) { if (src_cell.codepoint() == kitty.graphics.unicode.placeholder) {
dst_row.kitty_virtual_placeholder = true; dst_row.kitty_virtual_placeholder = true;
@ -1090,12 +1130,16 @@ pub const Page = struct {
/// Caller is responsible for updating the refcount in the hyperlink /// Caller is responsible for updating the refcount in the hyperlink
/// set as necessary by calling `use` if the id was not acquired with /// set as necessary by calling `use` if the id was not acquired with
/// `add`. /// `add`.
pub fn setHyperlink(self: *Page, row: *Row, cell: *Cell, id: hyperlink.Id) !void { pub fn setHyperlink(self: *Page, row: *Row, cell: *Cell, id: hyperlink.Id) error{HyperlinkMapOutOfMemory}!void {
defer self.assertIntegrity(); defer self.assertIntegrity();
const cell_offset = getOffset(Cell, self.memory, cell); const cell_offset = getOffset(Cell, self.memory, cell);
var map = self.hyperlink_map.map(self.memory); var map = self.hyperlink_map.map(self.memory);
const gop = try map.getOrPut(cell_offset); const gop = map.getOrPut(cell_offset) catch |e| {
comptime assert(@TypeOf(e) == error{OutOfMemory});
// The hyperlink map capacity needs to be increased.
return error.HyperlinkMapOutOfMemory;
};
if (gop.found_existing) { if (gop.found_existing) {
// Always release the old hyperlink, because even if it's actually // Always release the old hyperlink, because even if it's actually
@ -1160,7 +1204,7 @@ pub const Page = struct {
/// Set the graphemes for the given cell. This asserts that the cell /// Set the graphemes for the given cell. This asserts that the cell
/// has no graphemes set, and only contains a single codepoint. /// has no graphemes set, and only contains a single codepoint.
pub fn setGraphemes(self: *Page, row: *Row, cell: *Cell, cps: []u21) Allocator.Error!void { pub fn setGraphemes(self: *Page, row: *Row, cell: *Cell, cps: []u21) GraphemeError!void {
defer self.assertIntegrity(); defer self.assertIntegrity();
assert(cell.codepoint() > 0); assert(cell.codepoint() > 0);
@ -1169,14 +1213,22 @@ pub const Page = struct {
const cell_offset = getOffset(Cell, self.memory, cell); const cell_offset = getOffset(Cell, self.memory, cell);
var map = self.grapheme_map.map(self.memory); var map = self.grapheme_map.map(self.memory);
const slice = try self.grapheme_alloc.alloc(u21, self.memory, cps.len); const slice = self.grapheme_alloc.alloc(u21, self.memory, cps.len) catch |e| {
comptime assert(@TypeOf(e) == error{OutOfMemory});
// The grapheme alloc capacity needs to be increased.
return error.GraphemeAllocOutOfMemory;
};
errdefer self.grapheme_alloc.free(self.memory, slice); errdefer self.grapheme_alloc.free(self.memory, slice);
@memcpy(slice, cps); @memcpy(slice, cps);
try map.putNoClobber(cell_offset, .{ map.putNoClobber(cell_offset, .{
.offset = getOffset(u21, self.memory, @ptrCast(slice.ptr)), .offset = getOffset(u21, self.memory, @ptrCast(slice.ptr)),
.len = slice.len, .len = slice.len,
}); }) catch |e| {
comptime assert(@TypeOf(e) == error{OutOfMemory});
// The grapheme map capacity needs to be increased.
return error.GraphemeMapOutOfMemory;
};
errdefer map.remove(cell_offset); errdefer map.remove(cell_offset);
cell.content_tag = .codepoint_grapheme; cell.content_tag = .codepoint_grapheme;