mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-08-25 00:21:46 +00:00
apprt/gtk: add WeakRef.deinit and use it at teardown sites (#13732)
Fixes #13713. `WeakRef(T)` offers `set` and `get`, so releasing one is spelled `set(null)` — indistinguishable from an ordinary assignment. The requirement that it *must* happen before the owning memory is freed lives in a comment in `class/inspector_window.zig`, which is not where somebody using the type is looking. This adds `deinit`, forwarding to `g_weak_ref_clear` — the call GLib documents for a `GWeakRef` that is going away — and switches the dispose-time clears to it. ### What changed - `weak_ref.zig`: new `deinit`, with the reasoning in its doc comment. - `window.zig`, `split_tree.zig`, `application.zig`, `command_palette.zig`: the four dispose-time clears now call `deinit`. `set(null)` is unchanged and still valid. The clear in `Application.handleReloadConfig` deliberately stays a `set(null)`: the object is alive there and the reference is reused, so it is a logical clear rather than teardown — which is the distinction the new name is meant to make visible. ### Why it is worth a method Zig has no destructors, so this enforces nothing; it is documentation that happens to be executable. The concrete case is in #13713: I added a `WeakRef(Window)` in a downstream branch, did not clear it, and closing a window that had shown that dialog deadlocked the GTK main loop inside `weak_ref_data_clear_list` locking freed memory. Every upstream call site already gets this right — the point is only to put the rule where the next person will see it. ### Testing `zig build test` passes. `zig fmt --check` clean. Built and used on Linux/GTK; the change is behaviourally identical to what was there, since `g_weak_ref_clear` and `g_weak_ref_set(NULL)` both unregister. --- **AI disclosure per `AI_POLICY.md`:** I investigated the underlying incident with Claude Code and it drafted this change; I reviewed it.
This commit is contained in:
@@ -1557,7 +1557,7 @@ pub const Application = extern struct {
|
||||
diag.close();
|
||||
diag.unref(); // strong ref from get()
|
||||
}
|
||||
priv.config_errors_dialog.set(null);
|
||||
priv.config_errors_dialog.deinit();
|
||||
if (priv.signal_source) |v| {
|
||||
if (glib.Source.remove(v) == 0) {
|
||||
log.warn("unable to remove signal source", .{});
|
||||
|
||||
@@ -609,7 +609,7 @@ const Command = extern struct {
|
||||
switch (priv.data) {
|
||||
.regular => {},
|
||||
.jump => |*j| {
|
||||
j.surface.set(null);
|
||||
j.surface.deinit();
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
@@ -645,7 +645,7 @@ pub const SplitTree = extern struct {
|
||||
|
||||
fn dispose(self: *Self) callconv(.c) void {
|
||||
const priv = self.private();
|
||||
priv.last_focused.set(null);
|
||||
priv.last_focused.deinit();
|
||||
if (priv.rebuild_source) |v| {
|
||||
if (glib.Source.remove(v) == 0) {
|
||||
log.warn("unable to remove rebuild source", .{});
|
||||
|
||||
@@ -1329,7 +1329,7 @@ pub const Window = extern struct {
|
||||
priv.handle_active_state_source = null;
|
||||
}
|
||||
|
||||
priv.command_palette.set(null);
|
||||
priv.command_palette.deinit();
|
||||
|
||||
if (priv.config) |v| {
|
||||
v.unref();
|
||||
|
||||
@@ -22,6 +22,23 @@ pub fn WeakRef(comptime T: type) type {
|
||||
}
|
||||
}
|
||||
|
||||
/// Release this weak reference.
|
||||
///
|
||||
/// You MUST call this before the memory holding this struct is freed,
|
||||
/// which in practice means from the owner's `dispose`. The target keeps
|
||||
/// a pointer to this `GWeakRef` so that it can clear it when the target
|
||||
/// is finalized; if this memory is gone by then, the target walks into
|
||||
/// whatever now occupies it. That is an invalid read at best, and can
|
||||
/// hang: the target takes a lock inside each registered weak ref, and
|
||||
/// reused memory with the low bit set is a lock nothing will release.
|
||||
///
|
||||
/// `set(null)` also unregisters and remains valid. This exists so the
|
||||
/// requirement has a name at the use site rather than looking like an
|
||||
/// ordinary assignment.
|
||||
pub fn deinit(self: *Self) void {
|
||||
self.ref.clear();
|
||||
}
|
||||
|
||||
/// Get a strong reference to the object, or null if the object
|
||||
/// has been finalized. This increases the reference count by one.
|
||||
pub fn get(self: *Self) ?*T {
|
||||
|
||||
Reference in New Issue
Block a user