From 6cd684d5d3b2a83c9966b6c5ba239d36fbd937a9 Mon Sep 17 00:00:00 2001 From: Daniel Kinzler Date: Fri, 28 Aug 2026 14:25:33 +0200 Subject: [PATCH] gtk: fix stale pointers to property bindings Previously, the property binding created in `Surface.bindIsSplit` would get freed automatically when the source object (the SplitTree widget) got finalized. A subsequent call to `bindIsSplit` could then cause a crash by using the stale pointer to the binding. This bug could e.g. be triggered by dragging the surface from a single-surface tab to another tab. We now create an extra reference to the binding object so that Surface essentially owns the binding and is responsible for freeing it. Updated the binding in `SurfaceScrolledWindow` to use the same pattern. That one was probably fine, because the binding is only created once, but let's be safe. --- src/apprt/gtk/class/surface.zig | 20 +++++++++++++++++-- .../gtk/class/surface_scrolled_window.zig | 12 ++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/apprt/gtk/class/surface.zig b/src/apprt/gtk/class/surface.zig index e6cfe5c50..59d8ed0f7 100644 --- a/src/apprt/gtk/class/surface.zig +++ b/src/apprt/gtk/class/surface.zig @@ -853,14 +853,24 @@ pub const Surface = extern struct { pub fn bindIsSplit(self: *Self, tree: *SplitTree) void { const priv = self.private(); - if (priv.is_split_binding) |bind| bind.unbind(); + if (priv.is_split_binding) |binding| { + binding.unbind(); + binding.unref(); + priv.is_split_binding = null; + } - priv.is_split_binding = tree.as(gobject.Object).bindProperty( + const binding = tree.as(gobject.Object).bindProperty( "is-split", self.as(gobject.Object), "is-split", .{ .sync_create = true }, ); + // The ref created by bindProperty is owned by the binding itself. + // We need another ref to prevent the binding object from being + // freed if the source object (SplitTree) is finalized. Otherwise + // our pointer to the binding could become stale. + binding.ref(); + priv.is_split_binding = binding; } /// Callback used to determine whether unfocused-split-fill / unfocused-split-opacity @@ -1881,6 +1891,12 @@ pub const Surface = extern struct { priv.config = null; } + if (priv.is_split_binding) |binding| { + binding.unbind(); + binding.unref(); + priv.is_split_binding = null; + } + if (priv.vadj_signal_group) |group| { group.setTarget(null); group.as(gobject.Object).unref(); diff --git a/src/apprt/gtk/class/surface_scrolled_window.zig b/src/apprt/gtk/class/surface_scrolled_window.zig index 6ccb0a0d2..ad9a4cc7e 100644 --- a/src/apprt/gtk/class/surface_scrolled_window.zig +++ b/src/apprt/gtk/class/surface_scrolled_window.zig @@ -89,7 +89,8 @@ pub const SurfaceScrolledWindow = extern struct { const priv = self.private(); if (priv.config_binding) |binding| { - binding.as(gobject.Object).unref(); + binding.unbind(); + binding.unref(); priv.config_binding = null; } @@ -168,18 +169,23 @@ pub const SurfaceScrolledWindow = extern struct { // Unbind old config binding if it exists if (priv.config_binding) |binding| { - binding.as(gobject.Object).unref(); + binding.unbind(); + binding.unref(); priv.config_binding = null; } // Bind config from surface to our config property if (priv.surface) |surface| { - priv.config_binding = surface.as(gobject.Object).bindProperty( + const binding = surface.as(gobject.Object).bindProperty( properties.config.name, self.as(gobject.Object), properties.config.name, .{ .sync_create = true }, ); + // Keep another ref, otherwise the binding would be freed and + // our pointer become stale if the surface gets finalized. + binding.ref(); + priv.config_binding = binding; } }