Ensure X11 connections are closed (#3924)

Introduces a DisplayOwner struct to own both the library
and associated display pointer; their lifetimes are combined
in that struct.

The display pointer is encapsulated in a DisplayRef.

When DisplayOwner is dropped, it ensures that the DisplayRef
is correctly closed prior to unloading the library.

refs: https://github.com/gfx-rs/wgpu/issues/3813

CHANGELOG.md

@@ -75,6 +75,7 @@ Bottom level categories:
 - Fix order of arguments to glPolygonOffset by @komadori in [#3783](https://github.com/gfx-rs/wgpu/pull/3783).
 - Fix OpenGL/EGL backend not respecting non-sRGB texture formats in `SurfaceConfiguration`. by @liquidev in [#3817](https://github.com/gfx-rs/wgpu/pull/3817)
 - Make write- and read-only marked buffers match non-readonly layouts. by @fornwall in [#3893](https://github.com/gfx-rs/wgpu/pull/3893)
+- Fix leaking X11 connections. by @wez in [#3924](https://github.com/gfx-rs/wgpu/pull/3924)
 
 #### Metal
 

wgpu-hal/src/gles/egl.rs

@@ -21,6 +21,8 @@ const EGL_GL_COLORSPACE_SRGB_KHR: u32 = 0x3089;
 type XOpenDisplayFun =
     unsafe extern "system" fn(display_name: *const raw::c_char) -> *mut raw::c_void;
 
+type XCloseDisplayFun = unsafe extern "system" fn(display: *mut raw::c_void) -> raw::c_int;
+
 type WlDisplayConnectFun =
     unsafe extern "system" fn(display_name: *const raw::c_char) -> *mut raw::c_void;
 
@@ -112,13 +114,60 @@ unsafe extern "system" fn egl_debug_proc(
     );
 }
 
-fn open_x_display() -> Option<(ptr::NonNull<raw::c_void>, libloading::Library)> {
+/// A simple wrapper around an X11 or Wayland display handle.
+/// Since the logic in this file doesn't actually need to directly
+/// persist a wayland connection handle, the only load-bearing
+/// enum variant is the X11 variant
+#[derive(Debug)]
+enum DisplayRef {
+    X11(ptr::NonNull<raw::c_void>),
+    Wayland,
+}
+
+impl DisplayRef {
+    /// Convenience for getting the underlying pointer
+    fn as_ptr(&self) -> *mut raw::c_void {
+        match *self {
+            Self::X11(ptr) => ptr.as_ptr(),
+            Self::Wayland => unreachable!(),
+        }
+    }
+}
+
+/// DisplayOwner ties the lifetime of the system display handle
+/// to that of the loaded library.
+/// It implements Drop to ensure that the display handle is closed
+/// prior to unloading the library so that we don't leak the
+/// associated file descriptors
+#[derive(Debug)]
+struct DisplayOwner {
+    library: libloading::Library,
+    display: DisplayRef,
+}
+
+impl Drop for DisplayOwner {
+    fn drop(&mut self) {
+        match self.display {
+            DisplayRef::X11(ptr) => unsafe {
+                let func: libloading::Symbol<XCloseDisplayFun> =
+                    self.library.get(b"XCloseDisplay").unwrap();
+                func(ptr.as_ptr());
+            },
+            DisplayRef::Wayland => {}
+        }
+    }
+}
+
+fn open_x_display() -> Option<DisplayOwner> {
     log::info!("Loading X11 library to get the current display");
     unsafe {
         let library = libloading::Library::new("libX11.so").ok()?;
         let func: libloading::Symbol<XOpenDisplayFun> = library.get(b"XOpenDisplay").unwrap();
         let result = func(ptr::null());
-        ptr::NonNull::new(result).map(|ptr| (ptr, library))
+        ptr::NonNull::new(result).map(|ptr| DisplayOwner {
+            display: DisplayRef::X11(ptr),
+            library,
+        })
     }
 }
 
@@ -132,7 +181,7 @@ unsafe fn find_library(paths: &[&str]) -> Option<libloading::Library> {
     None
 }
 
-fn test_wayland_display() -> Option<libloading::Library> {
+fn test_wayland_display() -> Option<DisplayOwner> {
     /* We try to connect and disconnect here to simply ensure there
      * is an active wayland display available.
      */
@@ -147,7 +196,10 @@ fn test_wayland_display() -> Option<libloading::Library> {
         wl_display_disconnect(display.as_ptr());
         find_library(&["libwayland-egl.so.1", "libwayland-egl.so"])?
     };
-    Some(library)
+    Some(DisplayOwner {
+        library,
+        display: DisplayRef::Wayland,
+    })
 }
 
 #[derive(Clone, Copy, Debug)]
@@ -604,7 +656,7 @@ enum WindowKind {
 
 #[derive(Clone, Debug)]
 struct WindowSystemInterface {
-    library: Option<Arc<libloading::Library>>,
+    display_owner: Option<Arc<DisplayOwner>>,
     kind: WindowKind,
 }
 
@@ -705,59 +757,62 @@ impl crate::Instance<super::Api> for Instance {
         #[cfg(target_os = "emscripten")]
         let egl1_5: Option<&Arc<EglInstance>> = Some(&egl);
 
-        let (display, wsi_library, wsi_kind) = if let (Some(library), Some(egl)) =
-            (wayland_library, egl1_5)
-        {
-            log::info!("Using Wayland platform");
-            let display_attributes = [khronos_egl::ATTRIB_NONE];
-            let display = egl
-                .get_platform_display(
-                    EGL_PLATFORM_WAYLAND_KHR,
-                    khronos_egl::DEFAULT_DISPLAY,
-                    &display_attributes,
-                )
-                .unwrap();
-            (display, Some(Arc::new(library)), WindowKind::Wayland)
-        } else if let (Some((display, library)), Some(egl)) = (x11_display_library, egl1_5) {
-            log::info!("Using X11 platform");
-            let display_attributes = [khronos_egl::ATTRIB_NONE];
-            let display = egl
-                .get_platform_display(EGL_PLATFORM_X11_KHR, display.as_ptr(), &display_attributes)
-                .unwrap();
-            (display, Some(Arc::new(library)), WindowKind::X11)
-        } else if let (Some((display, library)), Some(egl)) = (angle_x11_display_library, egl1_5) {
-            log::info!("Using Angle platform with X11");
-            let display_attributes = [
-                EGL_PLATFORM_ANGLE_NATIVE_PLATFORM_TYPE_ANGLE as khronos_egl::Attrib,
-                EGL_PLATFORM_X11_KHR as khronos_egl::Attrib,
-                EGL_PLATFORM_ANGLE_DEBUG_LAYERS_ENABLED as khronos_egl::Attrib,
-                usize::from(desc.flags.contains(crate::InstanceFlags::VALIDATION)),
-                khronos_egl::ATTRIB_NONE,
-            ];
-            let display = egl
-                .get_platform_display(
-                    EGL_PLATFORM_ANGLE_ANGLE,
-                    display.as_ptr(),
-                    &display_attributes,
-                )
-                .unwrap();
-            (display, Some(Arc::new(library)), WindowKind::AngleX11)
-        } else if client_ext_str.contains("EGL_MESA_platform_surfaceless") {
-            log::info!("No windowing system present. Using surfaceless platform");
-            let egl = egl1_5.expect("Failed to get EGL 1.5 for surfaceless");
-            let display = egl
-                .get_platform_display(
-                    EGL_PLATFORM_SURFACELESS_MESA,
-                    std::ptr::null_mut(),
-                    &[khronos_egl::ATTRIB_NONE],
-                )
-                .unwrap();
-            (display, None, WindowKind::Unknown)
-        } else {
-            log::info!("EGL_MESA_platform_surfaceless not available. Using default platform");
-            let display = egl.get_display(khronos_egl::DEFAULT_DISPLAY).unwrap();
-            (display, None, WindowKind::Unknown)
-        };
+        let (display, display_owner, wsi_kind) =
+            if let (Some(library), Some(egl)) = (wayland_library, egl1_5) {
+                log::info!("Using Wayland platform");
+                let display_attributes = [khronos_egl::ATTRIB_NONE];
+                let display = egl
+                    .get_platform_display(
+                        EGL_PLATFORM_WAYLAND_KHR,
+                        khronos_egl::DEFAULT_DISPLAY,
+                        &display_attributes,
+                    )
+                    .unwrap();
+                (display, Some(Arc::new(library)), WindowKind::Wayland)
+            } else if let (Some(display_owner), Some(egl)) = (x11_display_library, egl1_5) {
+                log::info!("Using X11 platform");
+                let display_attributes = [khronos_egl::ATTRIB_NONE];
+                let display = egl
+                    .get_platform_display(
+                        EGL_PLATFORM_X11_KHR,
+                        display_owner.display.as_ptr(),
+                        &display_attributes,
+                    )
+                    .unwrap();
+                (display, Some(Arc::new(display_owner)), WindowKind::X11)
+            } else if let (Some(display_owner), Some(egl)) = (angle_x11_display_library, egl1_5) {
+                log::info!("Using Angle platform with X11");
+                let display_attributes = [
+                    EGL_PLATFORM_ANGLE_NATIVE_PLATFORM_TYPE_ANGLE as khronos_egl::Attrib,
+                    EGL_PLATFORM_X11_KHR as khronos_egl::Attrib,
+                    EGL_PLATFORM_ANGLE_DEBUG_LAYERS_ENABLED as khronos_egl::Attrib,
+                    usize::from(desc.flags.contains(crate::InstanceFlags::VALIDATION)),
+                    khronos_egl::ATTRIB_NONE,
+                ];
+                let display = egl
+                    .get_platform_display(
+                        EGL_PLATFORM_ANGLE_ANGLE,
+                        display_owner.display.as_ptr(),
+                        &display_attributes,
+                    )
+                    .unwrap();
+                (display, Some(Arc::new(display_owner)), WindowKind::AngleX11)
+            } else if client_ext_str.contains("EGL_MESA_platform_surfaceless") {
+                log::info!("No windowing system present. Using surfaceless platform");
+                let egl = egl1_5.expect("Failed to get EGL 1.5 for surfaceless");
+                let display = egl
+                    .get_platform_display(
+                        EGL_PLATFORM_SURFACELESS_MESA,
+                        std::ptr::null_mut(),
+                        &[khronos_egl::ATTRIB_NONE],
+                    )
+                    .unwrap();
+                (display, None, WindowKind::Unknown)
+            } else {
+                log::info!("EGL_MESA_platform_surfaceless not available. Using default platform");
+                let display = egl.get_display(khronos_egl::DEFAULT_DISPLAY).unwrap();
+                (display, None, WindowKind::Unknown)
+            };
 
         if desc.flags.contains(crate::InstanceFlags::VALIDATION)
             && client_ext_str.contains("EGL_KHR_debug")
@@ -785,7 +840,7 @@ impl crate::Instance<super::Api> for Instance {
 
         Ok(Instance {
             wsi: WindowSystemInterface {
-                library: wsi_library,
+                display_owner,
                 kind: wsi_kind,
             },
             flags: desc.flags,
@@ -1111,7 +1166,7 @@ impl crate::Surface<super::Api> for Surface {
                     }
                     (WindowKind::Unknown, Rwh::AndroidNdk(handle)) => handle.a_native_window,
                     (WindowKind::Wayland, Rwh::Wayland(handle)) => {
-                        let library = self.wsi.library.as_ref().unwrap();
+                        let library = &self.wsi.display_owner.as_ref().unwrap().library;
                         let wl_egl_window_create: libloading::Symbol<WlEglWindowCreateFun> =
                             unsafe { library.get(b"wl_egl_window_create") }.unwrap();
                         let window = unsafe { wl_egl_window_create(handle.surface, 640, 480) }
@@ -1215,7 +1270,7 @@ impl crate::Surface<super::Api> for Surface {
         };
 
         if let Some(window) = wl_window {
-            let library = self.wsi.library.as_ref().unwrap();
+            let library = &self.wsi.display_owner.as_ref().unwrap().library;
             let wl_egl_window_resize: libloading::Symbol<WlEglWindowResizeFun> =
                 unsafe { library.get(b"wl_egl_window_resize") }.unwrap();
             unsafe {
@@ -1281,7 +1336,12 @@ impl crate::Surface<super::Api> for Surface {
                 .destroy_surface(self.egl.display, surface)
                 .unwrap();
             if let Some(window) = wl_window {
-                let library = self.wsi.library.as_ref().expect("unsupported window");
+                let library = &self
+                    .wsi
+                    .display_owner
+                    .as_ref()
+                    .expect("unsupported window")
+                    .library;
                 let wl_egl_window_destroy: libloading::Symbol<WlEglWindowDestroyFun> =
                     unsafe { library.get(b"wl_egl_window_destroy") }.unwrap();
                 unsafe { wl_egl_window_destroy(window) };