avoid leaking resources on drop

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<Self> {
+    pub fn new_with(conn: &RawConnection, cmap: Colormap) -> Result<Self> {
         // 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<Self> {
+    pub fn new_with(conn: &RawConnection) -> Result<Self> {
         // 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<Cursor> {
+    fn load_cursor(conn: &RawConnection, font: Font, id: u16) -> Result<Cursor> {
         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<Item = xcb::Extension> + '_ {
         self.conn.active_extensions()
     }
 
+    /// Delegate for [`RawConnection::wait_for_reply`]
     pub fn wait_for_reply<C>(&self, cookie: C) -> xcb::Result<C::Reply>
     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);
+        }
     }
 }