libghostty: detach tracked grid refs on free (#12795)

Tracked grid references previously held a raw terminal wrapper pointer
and were required to be freed before the terminal. If callers kept one
past terminal destruction, later tracked-ref calls could dereference
freed terminal or page-list memory before detecting that the reference
was no longer meaningful.

Track live C tracked-grid-ref handles from the terminal wrapper and
detach them before tearing down terminal storage. Detached refs now
report no value through the tracked-ref APIs and can still be freed by
the caller. Update the C API docs to describe this lifetime behavior and
add a regression test for using a tracked ref after terminal free.

This introduces some overhead but tracked pins shouldn't be numerous and
this dramatically improves safety.

No API changes due to this (just more safety).
This commit is contained in:
Mitchell Hashimoto
2026-05-24 14:21:58 -07:00
committed by GitHub
6 changed files with 75 additions and 13 deletions

View File

@@ -70,8 +70,9 @@ extern "C" {
* the tracked reference between screens.
*
* Tracked references are owned by the caller and must be freed with
* ghostty_tracked_grid_ref_free() before the terminal that created them is
* freed.
* ghostty_tracked_grid_ref_free(). If the terminal that created a tracked
* reference is freed first, the handle remains valid only for tracked-grid-ref
* APIs: it reports no value and can still be freed.
*
* Each tracked reference adds bookkeeping to terminal mutations. Use them
* sparingly for long-lived anchors such as selections, search state, marks,
@@ -85,8 +86,8 @@ extern "C" {
* operation (including free).
*
* A tracked reference is allocated and must be freed when it is no
* longer needed. All tracked references must be freed prior to the
* terminal being freed.
* longer needed. A tracked reference may outlive the terminal that created it;
* after terminal free, it reports no value and can still be freed.
*
* ## Examples
*

View File

@@ -27,8 +27,8 @@ extern "C" {
/**
* Free a tracked grid reference.
*
* Passing NULL is allowed and has no effect. The reference must be freed before
* the terminal that created it is freed.
* Passing NULL is allowed and has no effect. A tracked reference may be freed
* after the terminal that created it is freed.
*
* @param ref Tracked grid reference to free.
*
@@ -39,6 +39,9 @@ GHOSTTY_API void ghostty_tracked_grid_ref_free(GhosttyTrackedGridRef ref);
/**
* Return whether a tracked grid reference currently has a meaningful value.
*
* If the terminal that created the tracked reference has been freed, this
* returns false.
*
* @param ref Tracked grid reference.
* @return true if the reference currently has a meaningful value.
*
@@ -62,7 +65,8 @@ GHOSTTY_API bool ghostty_tracked_grid_ref_has_value(
*
* If the tracked reference no longer has a meaningful value, this returns
* GHOSTTY_NO_VALUE. GHOSTTY_NO_VALUE is also returned when the reference cannot
* be represented in the requested coordinate space.
* be represented in the requested coordinate space, including after the
* terminal that created the tracked reference has been freed.
*
* @param ref Tracked grid reference.
* @param tag Coordinate space to convert into.
@@ -114,7 +118,8 @@ GHOSTTY_API GhosttyResult ghostty_tracked_grid_ref_set(
* ghostty_grid_ref_style().
*
* If the tracked reference no longer has a meaningful value, this returns
* GHOSTTY_NO_VALUE.
* GHOSTTY_NO_VALUE. This includes references whose owning terminal has been
* freed.
*
* @param ref Tracked grid reference.
* @param[out] out_ref On success, receives an untracked snapshot. May be NULL.

View File

@@ -1167,8 +1167,9 @@ GHOSTTY_API GhosttyResult ghostty_terminal_grid_ref(GhosttyTerminal terminal,
* If the point is outside the requested coordinate space, this returns
* GHOSTTY_INVALID_VALUE and writes NULL to out_ref.
*
* The returned handle must be freed with ghostty_tracked_grid_ref_free() before
* the terminal is freed.
* The returned handle must be freed with ghostty_tracked_grid_ref_free(). If
* the terminal is freed first, the handle remains valid only for
* tracked-grid-ref APIs: it reports no value and can still be freed.
*
* @param terminal Terminal instance.
* @param point Point to track.

View File

@@ -98,7 +98,9 @@ typedef struct GhosttyTerminalImpl* GhosttyTerminal;
* Opaque handle to a tracked grid reference.
*
* A tracked grid reference is owned by the caller and must be freed with
* ghostty_tracked_grid_ref_free() before the terminal that created it is freed.
* ghostty_tracked_grid_ref_free(). If the terminal that created it is freed
* first, the handle remains valid only for tracked-grid-ref APIs: it reports no
* value and can still be freed.
*
* @ingroup grid_ref
*/

View File

@@ -33,6 +33,9 @@ pub const TrackedGridRef = struct {
pub fn tracked_grid_ref_free(ref_: CTrackedGridRef) callconv(lib.calling_conv) void {
const ref = ref_ orelse return;
if (ref.terminal) |wrapper| {
_ = wrapper.tracked_grid_refs.swapRemove(ref);
}
if (ref.pageList()) |list| list.untrackPin(ref.pin);
ref.alloc.destroy(ref);
}
@@ -185,3 +188,38 @@ test "tracked_grid_ref reports no value after alternate screen reset" {
try testing.expect(tracked_grid_ref_has_value(ref));
try testing.expectEqual(Result.success, tracked_grid_ref_snapshot(ref, &snapshot));
}
test "tracked_grid_ref reports no value after terminal free" {
var terminal: terminal_c.Terminal = null;
try testing.expectEqual(Result.success, terminal_c.new(
&lib.alloc.test_allocator,
&terminal,
.{ .cols = 5, .rows = 2, .max_scrollback = 10_000 },
));
terminal_c.vt_write(terminal, "A", 1);
var ref: CTrackedGridRef = null;
try testing.expectEqual(Result.success, terminal_c.grid_ref_track(
terminal,
point.Point.cval(.{ .active = .{ .x = 0, .y = 0 } }),
&ref,
));
terminal_c.free(terminal);
try testing.expect(!tracked_grid_ref_has_value(ref));
var snapshot: grid_ref_c.CGridRef = undefined;
try testing.expectEqual(Result.no_value, tracked_grid_ref_snapshot(ref, &snapshot));
var coord: point.Coordinate = undefined;
try testing.expectEqual(Result.no_value, tracked_grid_ref_point(ref, .active, &coord));
try testing.expectEqual(Result.invalid_value, tracked_grid_ref_set(
ref,
terminal,
point.Point.cval(.{ .active = .{ .x = 0, .y = 0 } }),
));
tracked_grid_ref_free(ref);
}

View File

@@ -36,6 +36,7 @@ const TerminalWrapper = struct {
terminal: *ZigTerminal,
stream: Stream,
effects: Effects = .{},
tracked_grid_refs: std.AutoArrayHashMapUnmanaged(*grid_ref_tracked_c.TrackedGridRef, void) = .{},
};
/// C callback state for terminal effects. Trampolines are always
@@ -778,6 +779,18 @@ pub fn grid_ref_track(
.pin = tracked_pin,
};
// Store the tracked ref in the terminal so that when we free
// the terminal the tracked ref can be detached safely.
wrapper.tracked_grid_refs.putNoClobber(
alloc,
ref,
{},
) catch {
list.untrackPin(tracked_pin);
alloc.destroy(ref);
return .out_of_memory;
};
out.* = ref;
return .success;
}
@@ -799,9 +812,11 @@ pub fn point_from_grid_ref(
pub fn free(terminal_: Terminal) callconv(lib.calling_conv) void {
const wrapper = terminal_ orelse return;
const t = wrapper.terminal;
wrapper.stream.deinit();
const alloc = t.gpa();
for (wrapper.tracked_grid_refs.keys()) |ref| ref.terminal = null;
wrapper.tracked_grid_refs.deinit(alloc);
wrapper.stream.deinit();
t.deinit(alloc);
alloc.destroy(t);
alloc.destroy(wrapper);