Skip to content

Commit

Permalink
Ensure X11 connections are closed (#3924)
Browse files Browse the repository at this point in the history 
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: #3813
  • Loading branch information
@wez
wez authored Jul 13, 2023
1 parent 89f721f commit 2af6d1b
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 62 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
184 changes: 122 additions & 62 deletions wgpu-hal/src/gles/egl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
})
}
}

Expand All @@ -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.
*/
Expand All @@ -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)]
Expand Down Expand Up @@ -604,7 +656,7 @@ enum WindowKind {

#[derive(Clone, Debug)]
struct WindowSystemInterface {
library: Option<Arc<libloading::Library>>,
display_owner: Option<Arc<DisplayOwner>>,
kind: WindowKind,
}

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) }
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) };
Expand Down

0 comments on commit 2af6d1b

Please sign in to comment.