linux/x11: Do panic when unmapping/destroying of X11 window fails (#13262)

Thorsten Ball created 2 years ago

We saw this panic come up:

```
called `Result::unwrap()` on an `Err` value: IoError(Custom { kind: Other, error: UnknownError })
core::panicking::panic_fmt
core::result::unwrap_failed
<gpui::platform::linux::x11::window::X11Window as core::ops::drop::Drop>::drop
core::ptr::drop_in_place<gpui::platform::linux::x11::window::X11Window>
core::ptr::drop_in_place<gpui::window::Window>
gpui::app::AppContext::shutdown
gpui::app::AppContext::new::{{closure}}
gpui::platform::linux::platform::<impl gpui::platform::Platform for P>::run
gpui::app::App::run
zed::main
std::sys_common::backtrace::__rust_begin_short_backtrace
std::rt::lang_start::{{closure}}
std::rt::lang_start_internal
main
__libc_start_call_main
__libc_start_main_impl
_start
```

I'm not sure where exactly that error comes from, except from the X11
stuff. So let's be defensive and log error and only then tear down
everything.

I _think_ that if the error is repeatable that means we won't close the
window but instead just log errors, but I do think that's better than
panicking right now.

Release Notes:

- N/A

Change summary

crates/gpui/src/platform/linux/x11/window.rs | 46 ++++++++++++---------
1 file changed, 27 insertions(+), 19 deletions(-)

Detailed changes

crates/gpui/src/platform/linux/x11/window.rs

@@ -1,3 +1,5 @@
+use anyhow::Context;
+
 use crate::{
     platform::blade::{BladeRenderer, BladeSurfaceConfig},
     px, size, AnyWindowHandle, Bounds, DevicePixels, ForegroundExecutor, Modifiers, Pixels,
@@ -8,7 +10,7 @@ use crate::{
 
 use blade_graphics as gpu;
 use raw_window_handle as rwh;
-use util::ResultExt;
+use util::{maybe, ResultExt};
 use x11rb::{
     connection::Connection,
     protocol::{
@@ -422,26 +424,32 @@ impl Drop for X11Window {
         let mut state = self.0.state.borrow_mut();
         state.renderer.destroy();
 
-        self.0.xcb_connection.unmap_window(self.0.x_window).unwrap();
-        self.0
-            .xcb_connection
-            .destroy_window(self.0.x_window)
-            .unwrap();
-        self.0.xcb_connection.flush().unwrap();
+        let destroy_x_window = maybe!({
+            self.0.xcb_connection.unmap_window(self.0.x_window)?;
+            self.0.xcb_connection.destroy_window(self.0.x_window)?;
+            self.0.xcb_connection.flush()?;
 
-        // Mark window as destroyed so that we can filter out when X11 events
-        // for it still come in.
-        state.destroyed = true;
+            anyhow::Ok(())
+        })
+        .context("unmapping and destroying X11 window")
+        .log_err();
+
+        if destroy_x_window.is_some() {
+            // Mark window as destroyed so that we can filter out when X11 events
+            // for it still come in.
+            state.destroyed = true;
+
+            let this_ptr = self.0.clone();
+            let client_ptr = state.client.clone();
+            state
+                .executor
+                .spawn(async move {
+                    this_ptr.close();
+                    client_ptr.drop_window(this_ptr.x_window);
+                })
+                .detach();
+        }
 
-        let this_ptr = self.0.clone();
-        let client_ptr = state.client.clone();
-        state
-            .executor
-            .spawn(async move {
-                this_ptr.close();
-                client_ptr.drop_window(this_ptr.x_window);
-            })
-            .detach();
         drop(state);
     }
 }