From bce83081a79705e2fc463bee50aad52eb061c0be Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sat, 17 Aug 2024 15:59:43 +0200 Subject: [PATCH] Fix resource leak in cursor code The struct x11rb::cursor::Handle contained a cursor_font resource. This resource was never freed, so basically leaked. It's "just a font" and it is unlikely to cause a problem, but I want to deal with it anyway. One option would be to add a Drop implementation, but that would make everything more complicated since the Drop implementation would need access to the X11 connection. So, this basically requires an API break. In this commit, I go with the other option: Instead of keeping the core font open all the time, it is now only opened when needed and then immediately closed again. Since this is dealing with a font leak already, I am also changing the code from using a plain "Font" to the "FontWrapper" struct that cleans up the font in its Drop implementation. Signed-off-by: Uli Schlachter --- x11rb/src/cursor/mod.rs | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/x11rb/src/cursor/mod.rs b/x11rb/src/cursor/mod.rs index 35b8d428..a1ca7da7 100644 --- a/x11rb/src/cursor/mod.rs +++ b/x11rb/src/cursor/mod.rs @@ -6,7 +6,7 @@ use crate::connection::Connection; use crate::cookie::Cookie as X11Cookie; use crate::errors::{ConnectionError, ReplyOrIdError}; use crate::protocol::render::{self, Pictformat}; -use crate::protocol::xproto::{self, Font, Window}; +use crate::protocol::xproto::{self, FontWrapper, Window}; use crate::resource_manager::Database; use crate::NONE; use xcursor::parser::Image; @@ -30,7 +30,6 @@ enum RenderSupport { /// A cookie for creating a `Handle` #[derive(Debug)] pub struct Cookie<'a, 'b, C: Connection> { - conn: &'a C, screen: &'a xproto::Screen, resource_database: &'b Database, render_info: Option<( @@ -50,7 +49,6 @@ impl Cookie<'_, '_, C> { picture_format = find_format(&formats.reply()?); } Self::from_replies( - self.conn, self.screen, self.resource_database, render_version, @@ -72,7 +70,6 @@ impl Cookie<'_, '_, C> { } } Ok(Some(Self::from_replies( - self.conn, self.screen, self.resource_database, render_version, @@ -81,7 +78,6 @@ impl Cookie<'_, '_, C> { } fn from_replies( - conn: &C, screen: &xproto::Screen, resource_database: &Database, render_version: (u32, u32), @@ -106,11 +102,8 @@ impl Cookie<'_, '_, C> { _ => 0, }; let cursor_size = get_cursor_size(cursor_size, xft_dpi, screen); - let cursor_font = conn.generate_id()?; - let _ = xproto::open_font(conn, cursor_font, b"cursor")?; Ok(Handle { root: screen.root, - cursor_font, picture_format, render_support, theme, @@ -123,7 +116,6 @@ impl Cookie<'_, '_, C> { #[derive(Debug)] pub struct Handle { root: Window, - cursor_font: Font, picture_format: Pictformat, render_support: RenderSupport, theme: Option, @@ -158,7 +150,6 @@ impl Handle { None }; Ok(Cookie { - conn, screen, resource_database, render_info, @@ -184,15 +175,15 @@ fn open_cursor(theme: &Option, name: &str) -> Option( conn: &C, - cursor_font: Font, cursor: u16, ) -> Result { let result = conn.generate_id()?; + let cursor_font = FontWrapper::open_font(conn, b"cursor")?; let _ = xproto::create_glyph_cursor( conn, result, - cursor_font, - cursor_font, + cursor_font.font(), + cursor_font.font(), cursor, cursor + 1, // foreground color @@ -285,9 +276,7 @@ fn load_cursor( // Find the right cursor, load it directly if it is a core cursor let cursor_file = match open_cursor(&handle.theme, name) { None => return Ok(NONE), - Some(find_cursor::Cursor::CoreChar(c)) => { - return create_core_cursor(conn, handle.cursor_font, c) - } + Some(find_cursor::Cursor::CoreChar(c)) => return create_core_cursor(conn, c), Some(find_cursor::Cursor::File(f)) => f, };