From e38b963fa4cf6a8bb1341ba5437f8ed9ebe70730 Mon Sep 17 00:00:00 2001 From: tcmal Date: Fri, 21 Jun 2024 19:25:45 +0100 Subject: avoid leaking resources on drop --- src/conn_info/colours.rs | 16 +++++++++++++--- src/conn_info/cursors.rs | 17 +++++++++++++---- src/conn_info/mod.rs | 40 +++++++++++++++++++++++++++++++++------- 3 files changed, 59 insertions(+), 14 deletions(-) diff --git a/src/conn_info/colours.rs b/src/conn_info/colours.rs index 22b4fc2..d678bdb 100644 --- a/src/conn_info/colours.rs +++ b/src/conn_info/colours.rs @@ -1,7 +1,7 @@ use crate::error::Result; use xcb::{ - x::{AllocColor, Colormap}, - Connection, + x::{AllocColor, Colormap, FreeColors}, + Connection as RawConnection, }; /// Caches colours in an X11 color map. @@ -14,7 +14,7 @@ pub struct Colours { impl Colours { /// Load the colours into the given colour map - pub fn new_with(conn: &Connection, cmap: Colormap) -> Result { + pub fn new_with(conn: &RawConnection, cmap: Colormap) -> Result { // TODO: Move these colours out to config.rs let (border_normal, border_focused) = ( conn.wait_for_reply(conn.send_request(&AllocColor { @@ -47,4 +47,14 @@ impl Colours { pub const fn border_focused(&self) -> u32 { self.border_focused } + + /// Free the associated resources to avoid leaking them. + /// This object must not be used again after this method is called, as all resources will be invalid. + pub unsafe fn free(&self, conn: &RawConnection) { + conn.send_request(&FreeColors { + cmap: self.cmap, + plane_mask: 0, + pixels: &[self.border_normal, self.border_focused], + }); + } } diff --git a/src/conn_info/cursors.rs b/src/conn_info/cursors.rs index 119de5c..bb547e4 100644 --- a/src/conn_info/cursors.rs +++ b/src/conn_info/cursors.rs @@ -1,7 +1,7 @@ use crate::error::Result; use xcb::{ - x::{CreateGlyphCursor, Cursor, Font, OpenFont}, - Connection, + x::{CloseFont, CreateGlyphCursor, Cursor, Font, FreeCursor, OpenFont}, + Connection as RawConnection, }; // https://tronche.com/gui/x/xlib/appendix/b/ @@ -18,7 +18,7 @@ pub struct Cursors { impl Cursors { /// Load default cursors using the given connection. - pub fn new_with(conn: &Connection) -> Result { + pub fn new_with(conn: &RawConnection) -> Result { // Open cursor font let font = conn.generate_id(); conn.check_request(conn.send_request_checked(&OpenFont { @@ -33,7 +33,7 @@ impl Cursors { } /// Load the cursor with the given id from `font` - fn load_cursor(conn: &Connection, font: Font, id: u16) -> Result { + fn load_cursor(conn: &RawConnection, font: Font, id: u16) -> Result { let cid = conn.generate_id(); // https://github.com/mirror/libX11/blob/ff8706a5eae25b8bafce300527079f68a201d27f/src/Cursor.c#L34 conn.check_request(conn.send_request_checked(&CreateGlyphCursor { @@ -56,4 +56,13 @@ impl Cursors { pub const fn normal(&self) -> Cursor { self.normal } + + /// Free the associated resources to avoid leaking them. + /// This object must not be used again after this method is called, as all resources will be invalid. + pub unsafe fn free(&self, conn: &RawConnection) { + conn.send_request(&FreeCursor { + cursor: self.normal, + }); + conn.send_request(&CloseFont { font: self.font }); + } } diff --git a/src/conn_info/mod.rs b/src/conn_info/mod.rs index 623e2a5..521bf3e 100644 --- a/src/conn_info/mod.rs +++ b/src/conn_info/mod.rs @@ -157,11 +157,6 @@ impl<'a> Connection<'a> { }) } - /// Get the root window our WM is using - pub const fn root(&self) -> Window { - self.root - } - /// Refresh cached info about keyboard layout pub fn refresh_keyboard_info(&mut self) -> Result<()> { self.keyboard_state = KeyboardInfo::new_with(self.conn)?; @@ -194,10 +189,12 @@ impl<'a> Connection<'a> { self.conn.wait_for_event().map_err(Into::into) } + /// Delegate for [`RawConnection::active_extensions`] pub fn active_extensions(&self) -> impl Iterator + '_ { self.conn.active_extensions() } + /// Delegate for [`RawConnection::wait_for_reply`] pub fn wait_for_reply(&self, cookie: C) -> xcb::Result where C: xcb::CookieWithReplyChecked, @@ -205,10 +202,17 @@ impl<'a> Connection<'a> { self.conn.wait_for_reply(cookie) } + /// Delegate for [`RawConnection::get_setup`] pub fn get_setup(&self) -> &x::Setup { self.conn.get_setup() } + /// The root window + pub const fn root(&self) -> Window { + self.root + } + + /// The 'screen' number on the X server. Note this isn’t what you think it is on multi-monitor setups pub const fn screen_num(&self) -> usize { self.screen_num } @@ -219,7 +223,29 @@ impl Drop for Connection<'_> { self.send_request(&DestroyWindow { window: self.check_window, }); - // TODO: Unset attributes of root window - todo!() + + // Unset attributes of root window + self.conn.send_request(&DeleteProperty { + window: self.root, + property: self.atoms.net_wm_check, + }); + self.conn.send_request(&DeleteProperty { + window: self.root, + property: self.atoms.net_client_list, + }); + self.conn.send_request(&DeleteProperty { + window: self.root, + property: self.atoms.net_supported, + }); + self.conn.send_request(&ChangeWindowAttributes { + window: self.root, + value_list: &[x::Cw::EventMask(x::EventMask::NO_EVENT)], + }); + + // Safety: These methods only require that the object is not used again, which will be true since we're in the destructor. + unsafe { + self.cursors.free(self.conn); + self.colours.free(self.conn); + } } } -- cgit v1.2.3