From 1dbc8ca30c8ce929019c8a4d971113fc79cd4d58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20H=C3=A6gland?= Date: Mon, 10 Aug 2026 15:24:57 +0200 Subject: [PATCH] apprt/gtk: add WeakRef.deinit and use it at teardown sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A GWeakRef must be released before the memory holding it is freed: the target keeps a pointer to the GWeakRef so it can clear it at finalize, and if that memory is gone by then the target walks into whatever now occupies it. inspector_window.zig already carries this warning, and every call site follows it — but the rule lives in a comment in one file, while the type itself offers only set and get, so releasing one looks like an ordinary assignment. Give it a name. deinit forwards to g_weak_ref_clear, which is the call GLib documents for a GWeakRef that is going away, and the dispose-time clears now use it. set(null) still works and is unchanged; the clear in handleReloadConfig stays a set(null) because the object is still alive there and the reference is reused. Zig has no destructors so this enforces nothing. It puts the requirement on the type someone is already looking at. --- src/apprt/gtk/class/application.zig | 2 +- src/apprt/gtk/class/command_palette.zig | 2 +- src/apprt/gtk/class/split_tree.zig | 2 +- src/apprt/gtk/class/window.zig | 2 +- src/apprt/gtk/weak_ref.zig | 17 +++++++++++++++++ 5 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/apprt/gtk/class/application.zig b/src/apprt/gtk/class/application.zig index 14232f1f0..7ab7c9c20 100644 --- a/src/apprt/gtk/class/application.zig +++ b/src/apprt/gtk/class/application.zig @@ -1548,7 +1548,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", .{}); diff --git a/src/apprt/gtk/class/command_palette.zig b/src/apprt/gtk/class/command_palette.zig index 3ec7dfa0d..aaec00587 100644 --- a/src/apprt/gtk/class/command_palette.zig +++ b/src/apprt/gtk/class/command_palette.zig @@ -609,7 +609,7 @@ const Command = extern struct { switch (priv.data) { .regular => {}, .jump => |*j| { - j.surface.set(null); + j.surface.deinit(); }, } diff --git a/src/apprt/gtk/class/split_tree.zig b/src/apprt/gtk/class/split_tree.zig index 473ae3daf..24e6e70b6 100644 --- a/src/apprt/gtk/class/split_tree.zig +++ b/src/apprt/gtk/class/split_tree.zig @@ -647,7 +647,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", .{}); diff --git a/src/apprt/gtk/class/window.zig b/src/apprt/gtk/class/window.zig index f9e3b9841..caa7ddb4a 100644 --- a/src/apprt/gtk/class/window.zig +++ b/src/apprt/gtk/class/window.zig @@ -1296,7 +1296,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(); diff --git a/src/apprt/gtk/weak_ref.zig b/src/apprt/gtk/weak_ref.zig index f689e45fa..73cc6e511 100644 --- a/src/apprt/gtk/weak_ref.zig +++ b/src/apprt/gtk/weak_ref.zig @@ -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 {