From 863f8156f7660c47fbc612a1c24836aa7e8fd9c4 Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Wed, 28 Jul 2021 13:19:19 +0200 Subject: [PATCH] river: properly teardown surface tree When an xdg toplevel, layer surface, etc is destroyed, it is not guaranteed that all the children in the surface tree have already been destroyed. If there are still children around, destroying the root of the tree would leave dangling pointers. To fix this, destroy all children when destroying any node in the tree. --- river/DragIcon.zig | 2 + river/LayerSurface.zig | 6 +++ river/Subsurface.zig | 41 ++++++++++++++++----- river/XdgPopup.zig | 83 +++++++++++++++++++++++++----------------- river/XdgToplevel.zig | 10 +++-- 5 files changed, 96 insertions(+), 46 deletions(-) diff --git a/river/DragIcon.zig b/river/DragIcon.zig index e61d06f..bb05161 100644 --- a/river/DragIcon.zig +++ b/river/DragIcon.zig @@ -61,6 +61,8 @@ fn handleDestroy(listener: *wl.Listener(*wlr.Drag.Icon), wlr_drag_icon: *wlr.Dra drag_icon.unmap.link.remove(); drag_icon.new_subsurface.link.remove(); + Subsurface.destroySubsurfaces(wlr_drag_icon.surface); + util.gpa.destroy(node); } diff --git a/river/LayerSurface.zig b/river/LayerSurface.zig index 6f73c4d..68f2dc8 100644 --- a/river/LayerSurface.zig +++ b/river/LayerSurface.zig @@ -82,6 +82,12 @@ fn handleDestroy(listener: *wl.Listener(*wlr.LayerSurfaceV1), wlr_layer_surface: self.commit.link.remove(); self.new_subsurface.link.remove(); + Subsurface.destroySubsurfaces(self.wlr_layer_surface.surface); + var it = wlr_layer_surface.popups.iterator(.forward); + while (it.next()) |wlr_xdg_popup| { + if (@intToPtr(?*XdgPopup, wlr_xdg_popup.base.data)) |xdg_popup| xdg_popup.destroy(); + } + const node = @fieldParentPtr(std.TailQueue(Self).Node, "data", self); util.gpa.destroy(node); } diff --git a/river/Subsurface.zig b/river/Subsurface.zig index a5ae6e4..0880163 100644 --- a/river/Subsurface.zig +++ b/river/Subsurface.zig @@ -18,6 +18,7 @@ const Subsurface = @This(); const std = @import("std"); +const assert = std.debug.assert; const wlr = @import("wlroots"); const wl = @import("wayland").server.wl; @@ -26,16 +27,16 @@ const util = @import("util.zig"); const DragIcon = @import("DragIcon.zig"); const LayerSurface = @import("LayerSurface.zig"); -const View = @import("View.zig"); +const XdgToplevel = @import("XdgToplevel.zig"); pub const Parent = union(enum) { - view: *View, + xdg_toplevel: *XdgToplevel, layer_surface: *LayerSurface, drag_icon: *DragIcon, pub fn damageWholeOutput(parent: Parent) void { switch (parent) { - .view => |view| view.output.damage.addWhole(), + .xdg_toplevel => |xdg_toplevel| xdg_toplevel.view.output.damage.addWhole(), .layer_surface => |layer_surface| layer_surface.output.damage.addWhole(), .drag_icon => |drag_icon| { var it = server.root.outputs.first; @@ -50,7 +51,7 @@ parent: Parent, wlr_subsurface: *wlr.Subsurface, // Always active -destroy: wl.Listener(*wlr.Subsurface) = wl.Listener(*wlr.Subsurface).init(handleDestroy), +subsurface_destroy: wl.Listener(*wlr.Subsurface) = wl.Listener(*wlr.Subsurface).init(handleDestroy), map: wl.Listener(*wlr.Subsurface) = wl.Listener(*wlr.Subsurface).init(handleMap), unmap: wl.Listener(*wlr.Subsurface) = wl.Listener(*wlr.Subsurface).init(handleUnmap), new_subsurface: wl.Listener(*wlr.Subsurface) = wl.Listener(*wlr.Subsurface).init(handleNewSubsurface), @@ -65,8 +66,10 @@ pub fn create(wlr_subsurface: *wlr.Subsurface, parent: Parent) void { return; }; subsurface.* = .{ .wlr_subsurface = wlr_subsurface, .parent = parent }; + assert(wlr_subsurface.data == 0); + wlr_subsurface.data = @ptrToInt(subsurface); - wlr_subsurface.events.destroy.add(&subsurface.destroy); + wlr_subsurface.events.destroy.add(&subsurface.subsurface_destroy); wlr_subsurface.events.map.add(&subsurface.map); wlr_subsurface.events.unmap.add(&subsurface.unmap); wlr_subsurface.surface.events.new_subsurface.add(&subsurface.new_subsurface); @@ -85,17 +88,37 @@ pub fn handleExisting(surface: *wlr.Surface, parent: Parent) void { while (above_it.next()) |s| Subsurface.create(s, parent); } -fn handleDestroy(listener: *wl.Listener(*wlr.Subsurface), wlr_subsurface: *wlr.Subsurface) void { - const subsurface = @fieldParentPtr(Subsurface, "destroy", listener); - - subsurface.destroy.link.remove(); +/// Destroy this Subsurface and all of its children +pub fn destroy(subsurface: *Subsurface) void { + subsurface.subsurface_destroy.link.remove(); subsurface.map.link.remove(); subsurface.unmap.link.remove(); subsurface.new_subsurface.link.remove(); + Subsurface.destroySubsurfaces(subsurface.wlr_subsurface.surface); + + subsurface.wlr_subsurface.data = 0; util.gpa.destroy(subsurface); } +pub fn destroySubsurfaces(surface: *wlr.Surface) void { + var below_it = surface.subsurfaces_below.iterator(.forward); + while (below_it.next()) |wlr_subsurface| { + if (@intToPtr(?*Subsurface, wlr_subsurface.data)) |s| s.destroy(); + } + + var above_it = surface.subsurfaces_above.iterator(.forward); + while (above_it.next()) |wlr_subsurface| { + if (@intToPtr(?*Subsurface, wlr_subsurface.data)) |s| s.destroy(); + } +} + +fn handleDestroy(listener: *wl.Listener(*wlr.Subsurface), wlr_subsurface: *wlr.Subsurface) void { + const subsurface = @fieldParentPtr(Subsurface, "subsurface_destroy", listener); + + subsurface.destroy(); +} + fn handleMap(listener: *wl.Listener(*wlr.Subsurface), wlr_subsurface: *wlr.Subsurface) void { const subsurface = @fieldParentPtr(Subsurface, "map", listener); diff --git a/river/XdgPopup.zig b/river/XdgPopup.zig index e243b4b..dce546b 100644 --- a/river/XdgPopup.zig +++ b/river/XdgPopup.zig @@ -15,9 +15,10 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -const Self = @This(); +const XdgPopup = @This(); const std = @import("std"); +const assert = std.debug.assert; const wlr = @import("wlroots"); const wl = @import("wayland").server.wl; @@ -31,7 +32,7 @@ parent: Parent, wlr_xdg_popup: *wlr.XdgPopup, // Always active -destroy: wl.Listener(*wlr.XdgSurface) = wl.Listener(*wlr.XdgSurface).init(handleDestroy), +surface_destroy: wl.Listener(*wlr.XdgSurface) = wl.Listener(*wlr.XdgSurface).init(handleDestroy), map: wl.Listener(*wlr.XdgSurface) = wl.Listener(*wlr.XdgSurface).init(handleMap), unmap: wl.Listener(*wlr.XdgSurface) = wl.Listener(*wlr.XdgSurface).init(handleUnmap), new_popup: wl.Listener(*wlr.XdgPopup) = wl.Listener(*wlr.XdgPopup).init(handleNewPopup), @@ -41,28 +42,30 @@ new_subsurface: wl.Listener(*wlr.Subsurface) = wl.Listener(*wlr.Subsurface).init commit: wl.Listener(*wlr.Surface) = wl.Listener(*wlr.Surface).init(handleCommit), pub fn create(wlr_xdg_popup: *wlr.XdgPopup, parent: Parent) void { - const self = util.gpa.create(Self) catch { + const xdg_popup = util.gpa.create(XdgPopup) catch { std.log.crit("out of memory", .{}); wlr_xdg_popup.resource.postNoMemory(); return; }; - self.* = .{ + xdg_popup.* = .{ .parent = parent, .wlr_xdg_popup = wlr_xdg_popup, }; + assert(wlr_xdg_popup.base.data == 0); + wlr_xdg_popup.base.data = @ptrToInt(xdg_popup); const parent_box = switch (parent) { - .view => |view| &view.pending.box, + .xdg_toplevel => |xdg_toplevel| &xdg_toplevel.view.pending.box, .layer_surface => |layer_surface| &layer_surface.box, .drag_icon => unreachable, }; const output_dimensions = switch (parent) { - .view => |view| view.output.getEffectiveResolution(), + .xdg_toplevel => |xdg_toplevel| xdg_toplevel.view.output.getEffectiveResolution(), .layer_surface => |layer_surface| layer_surface.output.getEffectiveResolution(), .drag_icon => unreachable, }; - // The output box relative to the parent of the popup + // The output box relative to the parent of the xdg_popup var box = wlr.Box{ .x = -parent_box.x, .y = -parent_box.y, @@ -71,55 +74,69 @@ pub fn create(wlr_xdg_popup: *wlr.XdgPopup, parent: Parent) void { }; wlr_xdg_popup.unconstrainFromBox(&box); - wlr_xdg_popup.base.events.destroy.add(&self.destroy); - wlr_xdg_popup.base.events.map.add(&self.map); - wlr_xdg_popup.base.events.unmap.add(&self.unmap); - wlr_xdg_popup.base.events.new_popup.add(&self.new_popup); - wlr_xdg_popup.base.surface.events.new_subsurface.add(&self.new_subsurface); + wlr_xdg_popup.base.events.destroy.add(&xdg_popup.surface_destroy); + wlr_xdg_popup.base.events.map.add(&xdg_popup.map); + wlr_xdg_popup.base.events.unmap.add(&xdg_popup.unmap); + wlr_xdg_popup.base.events.new_popup.add(&xdg_popup.new_popup); + wlr_xdg_popup.base.surface.events.new_subsurface.add(&xdg_popup.new_subsurface); Subsurface.handleExisting(wlr_xdg_popup.base.surface, parent); } +pub fn destroy(xdg_popup: *XdgPopup) void { + xdg_popup.surface_destroy.link.remove(); + xdg_popup.map.link.remove(); + xdg_popup.unmap.link.remove(); + xdg_popup.new_popup.link.remove(); + xdg_popup.new_subsurface.link.remove(); + + Subsurface.destroySubsurfaces(xdg_popup.wlr_xdg_popup.base.surface); + XdgPopup.destroyPopups(xdg_popup.wlr_xdg_popup.base); + + xdg_popup.wlr_xdg_popup.base.data = 0; + util.gpa.destroy(xdg_popup); +} + +pub fn destroyPopups(wlr_xdg_surface: *wlr.XdgSurface) void { + var it = wlr_xdg_surface.popups.iterator(.forward); + while (it.next()) |wlr_xdg_popup| { + if (@intToPtr(?*XdgPopup, wlr_xdg_popup.base.data)) |xdg_popup| xdg_popup.destroy(); + } +} + fn handleDestroy(listener: *wl.Listener(*wlr.XdgSurface), wlr_xdg_surface: *wlr.XdgSurface) void { - const self = @fieldParentPtr(Self, "destroy", listener); - - self.destroy.link.remove(); - self.map.link.remove(); - self.unmap.link.remove(); - self.new_popup.link.remove(); - self.new_subsurface.link.remove(); - - util.gpa.destroy(self); + const xdg_popup = @fieldParentPtr(XdgPopup, "surface_destroy", listener); + xdg_popup.destroy(); } fn handleMap(listener: *wl.Listener(*wlr.XdgSurface), xdg_surface: *wlr.XdgSurface) void { - const self = @fieldParentPtr(Self, "map", listener); + const xdg_popup = @fieldParentPtr(XdgPopup, "map", listener); - self.wlr_xdg_popup.base.surface.events.commit.add(&self.commit); - self.parent.damageWholeOutput(); + xdg_popup.wlr_xdg_popup.base.surface.events.commit.add(&xdg_popup.commit); + xdg_popup.parent.damageWholeOutput(); } fn handleUnmap(listener: *wl.Listener(*wlr.XdgSurface), xdg_surface: *wlr.XdgSurface) void { - const self = @fieldParentPtr(Self, "unmap", listener); + const xdg_popup = @fieldParentPtr(XdgPopup, "unmap", listener); - self.commit.link.remove(); - self.parent.damageWholeOutput(); + xdg_popup.commit.link.remove(); + xdg_popup.parent.damageWholeOutput(); } fn handleCommit(listener: *wl.Listener(*wlr.Surface), surface: *wlr.Surface) void { - const self = @fieldParentPtr(Self, "commit", listener); + const xdg_popup = @fieldParentPtr(XdgPopup, "commit", listener); - self.parent.damageWholeOutput(); + xdg_popup.parent.damageWholeOutput(); } fn handleNewPopup(listener: *wl.Listener(*wlr.XdgPopup), wlr_xdg_popup: *wlr.XdgPopup) void { - const self = @fieldParentPtr(Self, "new_popup", listener); + const xdg_popup = @fieldParentPtr(XdgPopup, "new_popup", listener); - Self.create(wlr_xdg_popup, self.parent); + XdgPopup.create(wlr_xdg_popup, xdg_popup.parent); } fn handleNewSubsurface(listener: *wl.Listener(*wlr.Subsurface), new_wlr_subsurface: *wlr.Subsurface) void { - const self = @fieldParentPtr(Self, "new_subsurface", listener); + const xdg_popup = @fieldParentPtr(XdgPopup, "new_subsurface", listener); - Subsurface.create(new_wlr_subsurface, self.parent); + Subsurface.create(new_wlr_subsurface, xdg_popup.parent); } diff --git a/river/XdgToplevel.zig b/river/XdgToplevel.zig index bd55969..baddfb8 100644 --- a/river/XdgToplevel.zig +++ b/river/XdgToplevel.zig @@ -69,7 +69,7 @@ pub fn init(self: *Self, view: *View, xdg_surface: *wlr.XdgSurface) void { xdg_surface.events.new_popup.add(&self.new_popup); xdg_surface.surface.events.new_subsurface.add(&self.new_subsurface); - Subsurface.handleExisting(xdg_surface.surface, .{ .view = view }); + Subsurface.handleExisting(xdg_surface.surface, .{ .xdg_toplevel = self }); } pub fn deinit(self: *Self) void { @@ -80,6 +80,9 @@ pub fn deinit(self: *Self) void { self.unmap.link.remove(); self.new_popup.link.remove(); self.new_subsurface.link.remove(); + + Subsurface.destroySubsurfaces(self.xdg_surface.surface); + XdgPopup.destroyPopups(self.xdg_surface); } } @@ -157,7 +160,6 @@ pub fn getConstraints(self: Self) View.Constraints { }; } -/// Called when the xdg surface is destroyed fn handleDestroy(listener: *wl.Listener(*wlr.XdgSurface), xdg_surface: *wlr.XdgSurface) void { const self = @fieldParentPtr(Self, "destroy", listener); self.deinit(); @@ -298,12 +300,12 @@ fn handleCommit(listener: *wl.Listener(*wlr.Surface), surface: *wlr.Surface) voi fn handleNewPopup(listener: *wl.Listener(*wlr.XdgPopup), wlr_xdg_popup: *wlr.XdgPopup) void { const self = @fieldParentPtr(Self, "new_popup", listener); - XdgPopup.create(wlr_xdg_popup, .{ .view = self.view }); + XdgPopup.create(wlr_xdg_popup, .{ .xdg_toplevel = self }); } fn handleNewSubsurface(listener: *wl.Listener(*wlr.Subsurface), new_wlr_subsurface: *wlr.Subsurface) void { const self = @fieldParentPtr(Self, "new_subsurface", listener); - Subsurface.create(new_wlr_subsurface, .{ .view = self.view }); + Subsurface.create(new_wlr_subsurface, .{ .xdg_toplevel = self }); } /// Called when the client asks to be fullscreened. We always honor the request