mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-09-14 18:01:58 +00:00
config: rebuild RepeatableCommand's C mirror on clone (#14170)
## Why `RepeatableCommand.clone` copies `value_c` shallowly: the cloned `Command.C` structs keep string pointers into the *source* config's memory, so the clone only stays valid as long as its source lives. Every other field of a config clone is a deep copy — this is the one spot where the clone silently borrows. The macOS app never notices because of how it rotates configs: its clone's source is the core-owned live config, and both are replaced together on the next reload, so a clone never outlives its source. An embedder that clones a config and then frees the source — a legal sequence, e.g. promoting the clone to be the new active config — reads freed memory the next time it fetches `command-palette-entry` through the C API. Caught by AddressSanitizer. History: the shallow copy is as old as the C exposure itself. `dbe6035da` introduced `RepeatableCommand` with a correct deep `clone` (zig-side `value` only); `017021787` added the `value_c` mirror for `ghostty_config_get`, maintaining it carefully in `init`/`parseCLI` but extending `clone` with only the mechanical `ArrayList.clone` — the struct array copies, the string ownership doesn't. Nothing in-tree exercises clone-then-free-source, so it stayed latent. ## What Rebuild the C mirror from the cloned commands using the same `Command.cval` path `parseCLI` uses, so the clone's C strings live in the clone's own allocation. A regression test asserts the clone's C strings do not alias the source's while staying equal in content. ## AI disclosure Developed with AI assistance (Claude Code). The bug was found by AddressSanitizer while testing the Windows embedding host; the root-cause analysis, the fix, and the regression test were produced in an AI-assisted session, then reviewed and verified by the submitter (ASan clean after the fix, `RepeatableCommand` tests passing).
This commit is contained in:
@@ -8918,9 +8918,19 @@ pub const RepeatableCommand = struct {
|
||||
item.* = try item.clone(alloc);
|
||||
}
|
||||
|
||||
// Cloning value_c directly would copy Command.C structs
|
||||
// whose string pointers still reference the source config's
|
||||
// memory — the clone must stay valid after the source is
|
||||
// freed.
|
||||
var value_c: std.ArrayListUnmanaged(inputpkg.Command.C) = .empty;
|
||||
try value_c.ensureTotalCapacityPrecise(alloc, value.items.len);
|
||||
for (value.items) |item| {
|
||||
value_c.appendAssumeCapacity(try item.cval(alloc));
|
||||
}
|
||||
|
||||
return .{
|
||||
.value = value,
|
||||
.value_c = try self.value_c.clone(alloc),
|
||||
.value_c = value_c,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -9009,6 +9019,26 @@ pub const RepeatableCommand = struct {
|
||||
try testing.expectEqual(inputpkg.command.defaults.len, list.value.items.len);
|
||||
}
|
||||
|
||||
test "RepeatableCommand clone rebuilds the C mirror" {
|
||||
const testing = std.testing;
|
||||
var arena = ArenaAllocator.init(testing.allocator);
|
||||
defer arena.deinit();
|
||||
const alloc = arena.allocator();
|
||||
|
||||
var list: RepeatableCommand = .{};
|
||||
try list.parseCLI(alloc, "title:Foo,description:bar,action:new_tab");
|
||||
|
||||
const copy = try list.clone(alloc);
|
||||
try testing.expectEqual(list.value_c.items.len, copy.value_c.items.len);
|
||||
// The clone's C strings must not alias the source's — the
|
||||
// source config can be freed while the clone lives on.
|
||||
try testing.expect(list.value_c.items[0].title != copy.value_c.items[0].title);
|
||||
try testing.expectEqualStrings(
|
||||
std.mem.span(list.value_c.items[0].title),
|
||||
std.mem.span(copy.value_c.items[0].title),
|
||||
);
|
||||
}
|
||||
|
||||
test "RepeatableCommand formatConfig empty" {
|
||||
const testing = std.testing;
|
||||
var buf: std.Io.Writer.Allocating = .init(testing.allocator);
|
||||
|
||||
Reference in New Issue
Block a user