mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-05-25 06:18:37 +00:00
libghostty: detach tracked grid refs on free
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.
This commit is contained in:
@@ -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
|
||||
*
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -1134,8 +1134,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.
|
||||
|
||||
@@ -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
|
||||
*/
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -35,6 +35,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
|
||||
@@ -758,6 +759,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;
|
||||
}
|
||||
@@ -779,9 +792,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);
|
||||
|
||||
Reference in New Issue
Block a user