Skip to content

Commit

Permalink
Improve error handling and resource cleanup in linux/x11/window.rs
Browse files Browse the repository at this point in the history
* Fixes registration of event handler for xinput-2 device changes,
revealed by this improvement.

* Pushes `.unwrap()` panic-ing outwards to callers.

* Includes a description of what the X11 call was doing when a failure
was encountered.

* Fixes a variety of places where the X11 reply wasn't being inspected
for failures.

* Destroys windows on failure during setup. New structure makes it
possible for the caller of `open_window` to carry on despite failures,
and so partially initialized window should be removed (though all
calls I looked at also panic currently).

Considered pushing this through `linux/x11/client.rs` too but figured it'd be nice to minimize merge conflicts with #20853.
  • Loading branch information
mgsloan committed Nov 22, 2024
1 parent 14ea462 commit ed01141
Show file tree
Hide file tree
Showing 3 changed files with 452 additions and 347 deletions.
20 changes: 8 additions & 12 deletions crates/gpui/src/platform/linux/x11/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,11 +776,11 @@ impl X11Client {
},
};
let window = self.get_window(event.window)?;
window.configure(bounds);
window.configure(bounds).unwrap();
}
Event::PropertyNotify(event) => {
let window = self.get_window(event.window)?;
window.property_notify(event);
window.property_notify(event).unwrap();
}
Event::FocusIn(event) => {
let window = self.get_window(event.event)?;
Expand Down Expand Up @@ -1258,11 +1258,9 @@ impl LinuxClient for X11Client {
.iter()
.enumerate()
.filter_map(|(root_id, _)| {
Some(Rc::new(X11Display::new(
&state.xcb_connection,
state.scale_factor,
root_id,
)?) as Rc<dyn PlatformDisplay>)
Some(Rc::new(
X11Display::new(&state.xcb_connection, state.scale_factor, root_id).ok()?,
) as Rc<dyn PlatformDisplay>)
})
.collect()
}
Expand All @@ -1283,11 +1281,9 @@ impl LinuxClient for X11Client {
fn display(&self, id: DisplayId) -> Option<Rc<dyn PlatformDisplay>> {
let state = self.0.borrow();

Some(Rc::new(X11Display::new(
&state.xcb_connection,
state.scale_factor,
id.0 as usize,
)?))
Some(Rc::new(
X11Display::new(&state.xcb_connection, state.scale_factor, id.0 as usize).ok()?,
))
}

fn open_window(
Expand Down
13 changes: 9 additions & 4 deletions crates/gpui/src/platform/linux/x11/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,17 @@ pub(crate) struct X11Display {

impl X11Display {
pub(crate) fn new(
xc: &XCBConnection,
xcb: &XCBConnection,
scale_factor: f32,
x_screen_index: usize,
) -> Option<Self> {
let screen = xc.setup().roots.get(x_screen_index).unwrap();
Some(Self {
) -> anyhow::Result<Self> {
let Some(screen) = xcb.setup().roots.get(x_screen_index) else {
return Err(anyhow::anyhow!(
"No screen found with index {}",
x_screen_index
));
};
Ok(Self {
x_screen_index,
bounds: Bounds {
origin: Default::default(),
Expand Down
Loading

0 comments on commit ed01141

Please sign in to comment.