Skip to content

Commit

Permalink
Addressing memory leak in status bar
Browse files Browse the repository at this point in the history
I'm still not 100% confident that this is the last of the memory leak
issues in the status bar but at least locally for me this gets things
under control. There was a build up of un-acked expose / noexpose events
for the status bar being mapped that look to have been the remaining
source of leaked memory. Strangely, In working on this I repeatedly
ended up in a situation where x11rb was building up a large internal vec
(multiple MB) of what I _think_ was cookies for outstanding requests
that it had not received a reply for? I couldn't find any way of
inspecting that vec or forcing it to be cleared so if there continues to
be memory leak issues in this area I will likely need to reach out to
Psychon and check what it is I am missing here.
  • Loading branch information
sminez committed Jun 22, 2024
1 parent 2f67c78 commit cecad76
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 15 deletions.
1 change: 1 addition & 0 deletions crates/penrose_ui/src/bar/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ impl<X: XConn> StatusBar<X> {

self.draw.set_font(&self.font, ps.point_size)?;
let mut ctx = self.draw.context_for(id)?;
ctx.clear()?;

let mut extents = Vec::new();
let mut greedy_indices = Vec::new();
Expand Down
11 changes: 2 additions & 9 deletions crates/penrose_ui/src/core/fontset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,18 +248,11 @@ impl Font {
FcConfigSubstitute(std::ptr::null::<FcConfig>() as *mut _, pat, FcMatchPattern);
FcDefaultSubstitute(pat);

// https://doc.rust-lang.org/std/alloc/trait.GlobalAlloc.html#tymethod.alloc
let layout = Layout::new::<FcResult>();
let ptr = alloc(layout);
if ptr.is_null() {
handle_alloc_error(layout);
}
let res = ptr as *mut FcResult;
let mut res = FcResult::NoMatch;

// Passing the pointer from fontconfig_sys to x11 here
let fc_pattern = XftFontMatch(dpy, SCREEN, pat as *const _, res);
let fc_pattern = XftFontMatch(dpy, SCREEN, pat as *const _, &mut res as *mut _);

dealloc(res as *mut u8, layout);
FcCharSetDestroy(charset);
FcPatternDestroy(pat);

Expand Down
5 changes: 4 additions & 1 deletion crates/penrose_ui/src/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,15 @@ use x11::{
CapButt, Complex, CoordModeOrigin, Display, Drawable, False, JoinMiter, LineSolid, Window,
XCopyArea, XCreateGC, XCreatePixmap, XDefaultColormap, XDefaultDepth, XDefaultVisual,
XDrawRectangle, XFillPolygon, XFillRectangle, XFreeGC, XFreePixmap, XOpenDisplay, XPoint,
XSetForeground, XSetLineAttributes, XSync, GC,
XSetForeground, XSetGraphicsExposures, XSetLineAttributes, XSync, GC,
},
};

mod fontset;
use fontset::Fontset;

// Xlib manual: https://www.x.org/releases/current/doc/libX11/libX11/libX11.pdf

pub(crate) const SCREEN: i32 = 0;

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -203,6 +205,7 @@ impl Draw {
let drawable = XCreatePixmap(self.dpy, root, r.w, r.h, depth);
let gc = XCreateGC(self.dpy, root, 0, std::ptr::null_mut());
XSetLineAttributes(self.dpy, gc, 1, LineSolid, CapButt, JoinMiter);
XSetGraphicsExposures(self.dpy, gc, False);

(drawable, gc)
};
Expand Down
7 changes: 5 additions & 2 deletions src/builtin/actions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,12 @@ where

/// Exit penrose
///
/// Immediately exit the window manager with exit code 0.
/// Signal the `WindowManager` to exit it's main event loop.
pub fn exit<X: XConn>() -> Box<dyn KeyEventHandler<X>> {
key_handler(|_, _| std::process::exit(0))
key_handler(|s: &mut State<X>, _| {
s.running = false;
Ok(())
})
}

/// Info log the current window manager [State] for debugging purposes.
Expand Down
7 changes: 6 additions & 1 deletion src/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ where
pub(crate) pending_unmap: HashMap<Xid, usize>,
pub(crate) current_event: Option<XEvent>,
pub(crate) diff: Diff<Xid>,
pub(crate) running: bool,
// pub(crate) mouse_focused: bool,
// pub(crate) mouse_position: Option<(Point, Point)>,
}
Expand Down Expand Up @@ -111,6 +112,7 @@ where
pending_unmap: HashMap::new(),
current_event: None,
diff,
running: false,
})
}

Expand Down Expand Up @@ -472,8 +474,9 @@ where
}

manage_existing_clients(&mut self.state, &self.x)?;
self.state.running = true;

loop {
while self.state.running {
match self.x.next_event() {
Ok(event) => {
let span = span!(target: "penrose", Level::INFO, "XEvent", %event);
Expand All @@ -492,6 +495,8 @@ where
Err(e) => self.handle_error(e),
}
}

Ok(())
}

fn handle_xevent(&mut self, event: XEvent) -> Result<()> {
Expand Down
1 change: 1 addition & 0 deletions src/pure/stack_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,7 @@ impl StackSet<Xid> {
pending_unmap: Default::default(),
current_event: None,
diff: Default::default(),
running: false,
};

s.visible_client_positions(&crate::x::StubXConn)
Expand Down
4 changes: 2 additions & 2 deletions src/x11rb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,13 +385,13 @@ where
}

fn map(&self, client: Xid) -> Result<()> {
self.conn.map_window(*client)?;
self.conn.map_window(*client)?.ignore_error();

Ok(())
}

fn unmap(&self, client: Xid) -> Result<()> {
self.conn.unmap_window(*client)?;
self.conn.unmap_window(*client)?.ignore_error();

Ok(())
}
Expand Down

0 comments on commit cecad76

Please sign in to comment.