From 548d73519424c4d0a83ffcaadb4f1a9931545ec6 Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Tue, 11 Jul 2023 21:34:10 -0700 Subject: [PATCH] Ensure X11 connections are closed 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 --- wgpu-hal/src/gles/egl.rs | 184 ++++++++++++++++++++++++++------------- 1 file changed, 122 insertions(+), 62 deletions(-) diff --git a/wgpu-hal/src/gles/egl.rs b/wgpu-hal/src/gles/egl.rs index b9da38d45fb..0c8e754971d 100644 --- a/wgpu-hal/src/gles/egl.rs +++ b/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, 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), + 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 = + self.library.get(b"XCloseDisplay").unwrap(); + func(ptr.as_ptr()); + }, + DisplayRef::Wayland => {} + } + } +} + +fn open_x_display() -> Option { log::info!("Loading X11 library to get the current display"); unsafe { let library = libloading::Library::new("libX11.so").ok()?; let func: libloading::Symbol = 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 { None } -fn test_wayland_display() -> Option { +fn test_wayland_display() -> Option { /* 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 { 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>, + display_owner: Option>, kind: WindowKind, } @@ -705,59 +757,62 @@ impl crate::Instance for Instance { #[cfg(target_os = "emscripten")] let egl1_5: Option<&Arc> = 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 for Instance { Ok(Instance { wsi: WindowSystemInterface { - library: wsi_library, + display_owner, kind: wsi_kind, }, flags: desc.flags, @@ -1111,7 +1166,7 @@ impl crate::Surface 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 = 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 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 = unsafe { library.get(b"wl_egl_window_resize") }.unwrap(); unsafe { @@ -1281,7 +1336,12 @@ impl crate::Surface 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 = unsafe { library.get(b"wl_egl_window_destroy") }.unwrap(); unsafe { wl_egl_window_destroy(window) };