From dabf7e9e975ba6bdeed609e6195fba78c414f1f2 Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Thu, 14 Mar 2024 11:55:23 +0100 Subject: [PATCH] Server: clean up initialization This ended up uncovering a latent bug in the IdleInhibitorManager initialization code, not sure how that ever worked. --- river/IdleInhibitorManager.zig | 8 +- river/Server.zig | 223 +++++++++++++++++---------------- 2 files changed, 119 insertions(+), 112 deletions(-) diff --git a/river/IdleInhibitorManager.zig b/river/IdleInhibitorManager.zig index b64f9aa..ec7614d 100644 --- a/river/IdleInhibitorManager.zig +++ b/river/IdleInhibitorManager.zig @@ -12,12 +12,14 @@ const SceneNodeData = @import("SceneNodeData.zig"); const View = @import("View.zig"); idle_inhibit_manager: *wlr.IdleInhibitManagerV1, -new_idle_inhibitor: wl.Listener(*wlr.IdleInhibitorV1), +new_idle_inhibitor: wl.Listener(*wlr.IdleInhibitorV1) = + wl.Listener(*wlr.IdleInhibitorV1).init(handleNewIdleInhibitor), inhibitors: std.TailQueue(IdleInhibitor) = .{}, pub fn init(self: *Self) !void { - self.idle_inhibit_manager = try wlr.IdleInhibitManagerV1.create(server.wl_server); - self.new_idle_inhibitor.setNotify(handleNewIdleInhibitor); + self.* = .{ + .idle_inhibit_manager = try wlr.IdleInhibitManagerV1.create(server.wl_server), + }; self.idle_inhibit_manager.events.new_inhibitor.add(&self.new_idle_inhibitor); } diff --git a/river/Server.zig b/river/Server.zig index 91c6d78..d609de3 100644 --- a/river/Server.zig +++ b/river/Server.zig @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -const Self = @This(); +const Server = @This(); const build_options = @import("build_options"); const std = @import("std"); @@ -54,24 +54,33 @@ session: ?*wlr.Session, renderer: *wlr.Renderer, allocator: *wlr.Allocator, +compositor: *wlr.Compositor, + xdg_shell: *wlr.XdgShell, -new_xdg_surface: wl.Listener(*wlr.XdgSurface), +new_xdg_surface: wl.Listener(*wlr.XdgSurface) = + wl.Listener(*wlr.XdgSurface).init(handleNewXdgSurface), xdg_decoration_manager: *wlr.XdgDecorationManagerV1, -new_toplevel_decoration: wl.Listener(*wlr.XdgToplevelDecorationV1), +new_toplevel_decoration: wl.Listener(*wlr.XdgToplevelDecorationV1) = + wl.Listener(*wlr.XdgToplevelDecorationV1).init(handleNewToplevelDecoration), layer_shell: *wlr.LayerShellV1, -new_layer_surface: wl.Listener(*wlr.LayerSurfaceV1), +new_layer_surface: wl.Listener(*wlr.LayerSurfaceV1) = + wl.Listener(*wlr.LayerSurfaceV1).init(handleNewLayerSurface), -xwayland: if (build_options.xwayland) ?*wlr.Xwayland else void, -new_xwayland_surface: if (build_options.xwayland) wl.Listener(*wlr.XwaylandSurface) else void, +xwayland: if (build_options.xwayland) ?*wlr.Xwayland else void = if (build_options.xwayland) null, +new_xwayland_surface: if (build_options.xwayland) wl.Listener(*wlr.XwaylandSurface) else void = + if (build_options.xwayland) wl.Listener(*wlr.XwaylandSurface).init(handleNewXwaylandSurface), foreign_toplevel_manager: *wlr.ForeignToplevelManagerV1, xdg_activation: *wlr.XdgActivationV1, -request_activate: wl.Listener(*wlr.XdgActivationV1.event.RequestActivate), +request_activate: wl.Listener(*wlr.XdgActivationV1.event.RequestActivate) = + wl.Listener(*wlr.XdgActivationV1.event.RequestActivate).init(handleRequestActivate), -request_set_cursor_shape: wl.Listener(*wlr.CursorShapeManagerV1.event.RequestSetShape), +cursor_shape_manager: *wlr.CursorShapeManagerV1, +request_set_cursor_shape: wl.Listener(*wlr.CursorShapeManagerV1.event.RequestSetShape) = + wl.Listener(*wlr.CursorShapeManagerV1.event.RequestSetShape).init(handleRequestSetCursorShape), input_manager: InputManager, root: Root, @@ -82,155 +91,151 @@ layout_manager: LayoutManager, idle_inhibitor_manager: IdleInhibitorManager, lock_manager: LockManager, -pub fn init(self: *Self, runtime_xwayland: bool) !void { - self.wl_server = try wl.Server.create(); - errdefer self.wl_server.destroy(); +pub fn init(server: *Server, runtime_xwayland: bool) !void { + // We intentionally don't try to prevent memory leaks on error in this function + // since river will exit during initialization anyway if there is an error. + // This keeps the code simpler and more readable. - const loop = self.wl_server.getEventLoop(); - self.sigint_source = try loop.addSignal(*wl.Server, std.os.SIG.INT, terminate, self.wl_server); - errdefer self.sigint_source.remove(); - self.sigterm_source = try loop.addSignal(*wl.Server, std.os.SIG.TERM, terminate, self.wl_server); - errdefer self.sigterm_source.remove(); + const wl_server = try wl.Server.create(); - // This frees itself when the wl.Server is destroyed - self.backend = try wlr.Backend.autocreate(self.wl_server, &self.session); + var session: ?*wlr.Session = undefined; + const backend = try wlr.Backend.autocreate(wl_server, &session); + const renderer = try wlr.Renderer.autocreate(backend); - self.renderer = try wlr.Renderer.autocreate(self.backend); - errdefer self.renderer.destroy(); + const compositor = try wlr.Compositor.create(wl_server, 6, renderer); - try self.renderer.initWlShm(self.wl_server); + const loop = wl_server.getEventLoop(); + server.* = .{ + .wl_server = wl_server, + .sigint_source = try loop.addSignal(*wl.Server, std.os.SIG.INT, terminate, wl_server), + .sigterm_source = try loop.addSignal(*wl.Server, std.os.SIG.TERM, terminate, wl_server), - if (self.renderer.getDmabufFormats() != null and self.renderer.getDrmFd() >= 0) { + .backend = backend, + .session = session, + .renderer = renderer, + .allocator = try wlr.Allocator.autocreate(backend, renderer), + + .compositor = compositor, + .xdg_shell = try wlr.XdgShell.create(wl_server, 5), + .xdg_decoration_manager = try wlr.XdgDecorationManagerV1.create(wl_server), + .layer_shell = try wlr.LayerShellV1.create(wl_server, 4), + .foreign_toplevel_manager = try wlr.ForeignToplevelManagerV1.create(wl_server), + .xdg_activation = try wlr.XdgActivationV1.create(wl_server), + .cursor_shape_manager = try wlr.CursorShapeManagerV1.create(server.wl_server, 1), + + .config = try Config.init(), + + .root = undefined, + .input_manager = undefined, + .control = undefined, + .status_manager = undefined, + .layout_manager = undefined, + .idle_inhibitor_manager = undefined, + .lock_manager = undefined, + }; + + try renderer.initWlShm(wl_server); + if (renderer.getDmabufFormats() != null and renderer.getDrmFd() >= 0) { // wl_drm is a legacy interface and all clients should switch to linux_dmabuf. // However, enough widely used clients still rely on wl_drm that the pragmatic option // is to keep it around for the near future. // TODO remove wl_drm support - _ = try wlr.Drm.create(self.wl_server, self.renderer); + _ = try wlr.Drm.create(wl_server, renderer); - _ = try wlr.LinuxDmabufV1.createWithRenderer(self.wl_server, 4, self.renderer); + _ = try wlr.LinuxDmabufV1.createWithRenderer(wl_server, 4, renderer); } - self.allocator = try wlr.Allocator.autocreate(self.backend, self.renderer); - errdefer self.allocator.destroy(); + _ = try wlr.Subcompositor.create(wl_server); + _ = try wlr.PrimarySelectionDeviceManagerV1.create(wl_server); + _ = try wlr.DataDeviceManager.create(wl_server); + _ = try wlr.DataControlManagerV1.create(wl_server); + _ = try wlr.ExportDmabufManagerV1.create(wl_server); + _ = try wlr.ScreencopyManagerV1.create(wl_server); + _ = try wlr.SinglePixelBufferManagerV1.create(wl_server); + _ = try wlr.Viewporter.create(wl_server); + _ = try wlr.FractionalScaleManagerV1.create(wl_server, 1); - const compositor = try wlr.Compositor.create(self.wl_server, 6, self.renderer); - _ = try wlr.Subcompositor.create(self.wl_server); - - self.xdg_shell = try wlr.XdgShell.create(self.wl_server, 5); - self.new_xdg_surface.setNotify(handleNewXdgSurface); - self.xdg_shell.events.new_surface.add(&self.new_xdg_surface); - - self.xdg_decoration_manager = try wlr.XdgDecorationManagerV1.create(self.wl_server); - self.new_toplevel_decoration.setNotify(handleNewToplevelDecoration); - self.xdg_decoration_manager.events.new_toplevel_decoration.add(&self.new_toplevel_decoration); - - self.layer_shell = try wlr.LayerShellV1.create(self.wl_server, 4); - self.new_layer_surface.setNotify(handleNewLayerSurface); - self.layer_shell.events.new_surface.add(&self.new_layer_surface); - - if (build_options.xwayland) { - if (runtime_xwayland) { - self.xwayland = try wlr.Xwayland.create(self.wl_server, compositor, false); - self.new_xwayland_surface.setNotify(handleNewXwaylandSurface); - self.xwayland.?.events.new_surface.add(&self.new_xwayland_surface); - } else { - self.xwayland = null; - } + if (build_options.xwayland and runtime_xwayland) { + server.xwayland = try wlr.Xwayland.create(wl_server, compositor, false); + server.xwayland.?.events.new_surface.add(&server.new_xwayland_surface); } - self.foreign_toplevel_manager = try wlr.ForeignToplevelManagerV1.create(self.wl_server); + try server.root.init(); + try server.input_manager.init(); + try server.control.init(); + try server.status_manager.init(); + try server.layout_manager.init(); + try server.idle_inhibitor_manager.init(); + try server.lock_manager.init(); - self.xdg_activation = try wlr.XdgActivationV1.create(self.wl_server); - self.xdg_activation.events.request_activate.add(&self.request_activate); - self.request_activate.setNotify(handleRequestActivate); + server.xdg_shell.events.new_surface.add(&server.new_xdg_surface); + server.xdg_decoration_manager.events.new_toplevel_decoration.add(&server.new_toplevel_decoration); + server.layer_shell.events.new_surface.add(&server.new_layer_surface); + server.xdg_activation.events.request_activate.add(&server.request_activate); + server.cursor_shape_manager.events.request_set_shape.add(&server.request_set_cursor_shape); - _ = try wlr.PrimarySelectionDeviceManagerV1.create(self.wl_server); - - self.config = try Config.init(); - try self.root.init(); - // Must be called after root is initialized - try self.input_manager.init(); - try self.control.init(); - try self.status_manager.init(); - try self.layout_manager.init(); - try self.idle_inhibitor_manager.init(); - try self.lock_manager.init(); - - const cursor_shape_manager = try wlr.CursorShapeManagerV1.create(self.wl_server, 1); - self.request_set_cursor_shape.setNotify(handleRequestSetCursorShape); - cursor_shape_manager.events.request_set_shape.add(&self.request_set_cursor_shape); - - // These all free themselves when the wl_server is destroyed - _ = try wlr.DataDeviceManager.create(self.wl_server); - _ = try wlr.DataControlManagerV1.create(self.wl_server); - _ = try wlr.ExportDmabufManagerV1.create(self.wl_server); - _ = try wlr.ScreencopyManagerV1.create(self.wl_server); - _ = try wlr.SinglePixelBufferManagerV1.create(self.wl_server); - _ = try wlr.Viewporter.create(self.wl_server); - _ = try wlr.FractionalScaleManagerV1.create(self.wl_server, 1); - - self.wl_server.setGlobalFilter(*Self, globalFilter, self); + wl_server.setGlobalFilter(*Server, globalFilter, server); } /// Free allocated memory and clean up. Note: order is important here -pub fn deinit(self: *Self) void { - self.sigint_source.remove(); - self.sigterm_source.remove(); +pub fn deinit(server: *Server) void { + server.sigint_source.remove(); + server.sigterm_source.remove(); - self.new_xdg_surface.link.remove(); - self.new_layer_surface.link.remove(); - self.request_activate.link.remove(); + server.new_xdg_surface.link.remove(); + server.new_layer_surface.link.remove(); + server.request_activate.link.remove(); if (build_options.xwayland) { - if (self.xwayland) |xwayland| { - self.new_xwayland_surface.link.remove(); + if (server.xwayland) |xwayland| { + server.new_xwayland_surface.link.remove(); xwayland.destroy(); } } - self.wl_server.destroyClients(); + server.wl_server.destroyClients(); - self.backend.destroy(); + server.backend.destroy(); // The scene graph needs to be destroyed after the backend but before the renderer // Output destruction requires the scene graph to still be around while the scene // graph may require the renderer to still be around to destroy textures it seems. - self.root.scene.tree.node.destroy(); + server.root.scene.tree.node.destroy(); - self.renderer.destroy(); - self.allocator.destroy(); + server.renderer.destroy(); + server.allocator.destroy(); - self.root.deinit(); - self.input_manager.deinit(); - self.idle_inhibitor_manager.deinit(); - self.lock_manager.deinit(); + server.root.deinit(); + server.input_manager.deinit(); + server.idle_inhibitor_manager.deinit(); + server.lock_manager.deinit(); - self.wl_server.destroy(); + server.wl_server.destroy(); - self.config.deinit(); + server.config.deinit(); } /// Create the socket, start the backend, and setup the environment -pub fn start(self: Self) !void { +pub fn start(server: Server) !void { var buf: [11]u8 = undefined; - const socket = try self.wl_server.addSocketAuto(&buf); - try self.backend.start(); + const socket = try server.wl_server.addSocketAuto(&buf); + try server.backend.start(); // TODO: don't use libc's setenv if (c.setenv("WAYLAND_DISPLAY", socket.ptr, 1) < 0) return error.SetenvError; if (build_options.xwayland) { - if (self.xwayland) |xwayland| { + if (server.xwayland) |xwayland| { if (c.setenv("DISPLAY", xwayland.display_name, 1) < 0) return error.SetenvError; } } } -fn globalFilter(client: *const wl.Client, global: *const wl.Global, self: *Self) bool { +fn globalFilter(client: *const wl.Client, global: *const wl.Global, server: *Server) bool { // Only expose the xwalyand_shell_v1 global to the Xwayland process. if (build_options.xwayland) { - if (self.xwayland) |xwayland| { + if (server.xwayland) |xwayland| { if (global == xwayland.shell_v1.global) { - if (xwayland.server) |server| { - return client == server.client; + if (xwayland.server) |xwayland_server| { + return client == xwayland_server.client; } return false; } @@ -269,7 +274,7 @@ fn handleNewToplevelDecoration( } fn handleNewLayerSurface(listener: *wl.Listener(*wlr.LayerSurfaceV1), wlr_layer_surface: *wlr.LayerSurfaceV1) void { - const self = @fieldParentPtr(Self, "new_layer_surface", listener); + const server = @fieldParentPtr(Server, "new_layer_surface", listener); log.debug( "new layer surface: namespace {s}, layer {s}, anchor {b:0>4}, size {},{}, margin {},{},{},{}, exclusive_zone {}", @@ -290,7 +295,7 @@ fn handleNewLayerSurface(listener: *wl.Listener(*wlr.LayerSurfaceV1), wlr_layer_ // If the new layer surface does not have an output assigned to it, use the // first output or close the surface if none are available. if (wlr_layer_surface.output == null) { - const output = self.input_manager.defaultSeat().focused_output orelse { + const output = server.input_manager.defaultSeat().focused_output orelse { log.err("no output available for layer surface '{s}'", .{wlr_layer_surface.namespace}); wlr_layer_surface.destroy(); return; @@ -329,7 +334,7 @@ fn handleRequestActivate( listener: *wl.Listener(*wlr.XdgActivationV1.event.RequestActivate), event: *wlr.XdgActivationV1.event.RequestActivate, ) void { - const server = @fieldParentPtr(Self, "request_activate", listener); + const server = @fieldParentPtr(Server, "request_activate", listener); const node_data = SceneNodeData.fromSurface(event.surface) orelse return; switch (node_data.data) {