Skip to content

Commit

Permalink
Fix resource leak in cursor code
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
psychon committed Aug 17, 2024
1 parent c21935c commit bce8308
Showing 1 changed file with 5 additions and 16 deletions.
21 changes: 5 additions & 16 deletions x11rb/src/cursor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<(
Expand All @@ -50,7 +49,6 @@ impl<C: Connection> Cookie<'_, '_, C> {
picture_format = find_format(&formats.reply()?);
}
Self::from_replies(
self.conn,
self.screen,
self.resource_database,
render_version,
Expand All @@ -72,7 +70,6 @@ impl<C: Connection> Cookie<'_, '_, C> {
}
}
Ok(Some(Self::from_replies(
self.conn,
self.screen,
self.resource_database,
render_version,
Expand All @@ -81,7 +78,6 @@ impl<C: Connection> Cookie<'_, '_, C> {
}

fn from_replies(
conn: &C,
screen: &xproto::Screen,
resource_database: &Database,
render_version: (u32, u32),
Expand All @@ -106,11 +102,8 @@ impl<C: Connection> 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,
Expand All @@ -123,7 +116,6 @@ impl<C: Connection> Cookie<'_, '_, C> {
#[derive(Debug)]
pub struct Handle {
root: Window,
cursor_font: Font,
picture_format: Pictformat,
render_support: RenderSupport,
theme: Option<String>,
Expand Down Expand Up @@ -158,7 +150,6 @@ impl Handle {
None
};
Ok(Cookie {
conn,
screen,
resource_database,
render_info,
Expand All @@ -184,15 +175,15 @@ fn open_cursor(theme: &Option<String>, name: &str) -> Option<find_cursor::Cursor

fn create_core_cursor<C: Connection>(
conn: &C,
cursor_font: Font,
cursor: u16,
) -> Result<xproto::Cursor, ReplyOrIdError> {
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
Expand Down Expand Up @@ -285,9 +276,7 @@ fn load_cursor<C: Connection>(
// 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,
};

Expand Down

0 comments on commit bce8308

Please sign in to comment.