gles: fix crash when holding multiple devices on wayland/surfaceless. (…

…#5351)
gfx-rs · Mar 10, 2024 · 8e15707 · 8e15707
1 parent f78e2f7
commit 8e15707
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -41,7 +41,7 @@ Bottom level categories:
 
 ### Major Changes
 
-#### Vendored WebGPU Bindings from `web_sys` 
+#### Vendored WebGPU Bindings from `web_sys`
 
 **`--cfg=web_sys_unstable_apis` is no longer needed in your `RUSTFLAGS` to compile for WebGPU!!!**
 
@@ -165,6 +165,7 @@ By @cwfitzgerald in [#5325](https://github.com/gfx-rs/wgpu/pull/5325).
 
 - Fixes for being able to use an OpenGL 4.1 core context provided by macOS with wgpu. By @bes in [#5331](https://github.com/gfx-rs/wgpu/pull/5331).
 - Don't create a program for shader-clearing if that workaround isn't required. By @Dinnerbone in [#5348](https://github.com/gfx-rs/wgpu/pull/5348).
+- Fix crash when holding multiple devices on wayland/surfaceless. By @ashdnazg in [#5351](https://github.com/gfx-rs/wgpu/pull/5351).
 
 #### Vulkan
 

diff --git a/tests/tests/device.rs b/tests/tests/device.rs
@@ -29,39 +29,69 @@ static CROSS_DEVICE_BIND_GROUP_USAGE: GpuTestConfiguration = GpuTestConfiguratio
     });
 
 #[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
-#[test]
-fn device_lifetime_check() {
-    use pollster::FutureExt as _;
-
-    env_logger::init();
-    let instance = wgpu::Instance::new(wgpu::InstanceDescriptor {
-        backends: wgpu::util::backend_bits_from_env().unwrap_or(wgpu::Backends::all()),
-        dx12_shader_compiler: wgpu::util::dx12_shader_compiler_from_env().unwrap_or_default(),
-        gles_minor_version: wgpu::util::gles_minor_version_from_env().unwrap_or_default(),
-        flags: wgpu::InstanceFlags::advanced_debugging().with_env(),
-    });
+#[gpu_test]
+static DEVICE_LIFETIME_CHECK: GpuTestConfiguration = GpuTestConfiguration::new()
+    .parameters(TestParameters::default())
+    .run_sync(|_| {
+        use pollster::FutureExt as _;
+
+        let instance = wgpu::Instance::new(wgpu::InstanceDescriptor {
+            backends: wgpu::util::backend_bits_from_env().unwrap_or(wgpu::Backends::all()),
+            dx12_shader_compiler: wgpu::util::dx12_shader_compiler_from_env().unwrap_or_default(),
+            gles_minor_version: wgpu::util::gles_minor_version_from_env().unwrap_or_default(),
+            flags: wgpu::InstanceFlags::advanced_debugging().with_env(),
+        });
 
-    let adapter = wgpu::util::initialize_adapter_from_env_or_default(&instance, None)
-        .block_on()
-        .expect("failed to create adapter");
+        let adapter = wgpu::util::initialize_adapter_from_env_or_default(&instance, None)
+            .block_on()
+            .expect("failed to create adapter");
 
-    let (device, queue) = adapter
-        .request_device(&wgpu::DeviceDescriptor::default(), None)
-        .block_on()
-        .expect("failed to create device");
+        let (device, queue) = adapter
+            .request_device(&wgpu::DeviceDescriptor::default(), None)
+            .block_on()
+            .expect("failed to create device");
 
-    instance.poll_all(false);
+        instance.poll_all(false);
 
-    let pre_report = instance.generate_report().unwrap().unwrap();
+        let pre_report = instance.generate_report().unwrap();
 
-    drop(queue);
-    drop(device);
-    let post_report = instance.generate_report().unwrap().unwrap();
-    assert_ne!(
-        pre_report, post_report,
-        "Queue and Device has not been dropped as expected"
-    );
-}
+        drop(queue);
+        drop(device);
+        let post_report = instance.generate_report().unwrap();
+        assert_ne!(
+            pre_report, post_report,
+            "Queue and Device has not been dropped as expected"
+        );
+    });
+
+#[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
+#[gpu_test]
+static MULTIPLE_DEVICES: GpuTestConfiguration = GpuTestConfiguration::new()
+    .parameters(TestParameters::default())
+    .run_sync(|_| {
+        use pollster::FutureExt as _;
+
+        fn create_device_and_queue() -> (wgpu::Device, wgpu::Queue) {
+            let instance = wgpu::Instance::new(wgpu::InstanceDescriptor {
+                backends: wgpu::util::backend_bits_from_env().unwrap_or(wgpu::Backends::all()),
+                dx12_shader_compiler: wgpu::util::dx12_shader_compiler_from_env()
+                    .unwrap_or_default(),
+                gles_minor_version: wgpu::util::gles_minor_version_from_env().unwrap_or_default(),
+                flags: wgpu::InstanceFlags::advanced_debugging().with_env(),
+            });
+
+            let adapter = wgpu::util::initialize_adapter_from_env_or_default(&instance, None)
+                .block_on()
+                .expect("failed to create adapter");
+
+            adapter
+                .request_device(&wgpu::DeviceDescriptor::default(), None)
+                .block_on()
+                .expect("failed to create device")
+        }
+
+        let _ = vec![create_device_and_queue(), create_device_and_queue()];
+    });
 
 #[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
 #[gpu_test]

diff --git a/wgpu-hal/src/gles/egl.rs b/wgpu-hal/src/gles/egl.rs
@@ -1,7 +1,8 @@
 use glow::HasContext;
+use once_cell::sync::Lazy;
 use parking_lot::{Mutex, MutexGuard, RwLock};
 
-use std::{ffi, os::raw, ptr, rc::Rc, sync::Arc, time::Duration};
+use std::{collections::HashMap, ffi, os::raw, ptr, rc::Rc, sync::Arc, time::Duration};
 
 /// The amount of time to wait while trying to obtain a lock to the adapter context
 const CONTEXT_LOCK_TIMEOUT_SECS: u64 = 1;
@@ -432,14 +433,53 @@ struct Inner {
     srgb_kind: SrgbFrameBufferKind,
 }
 
+// Different calls to `eglGetPlatformDisplay` may return the same `Display`, making it a global
+// state of all our `EglContext`s. This forces us to track the number of such context to prevent
+// terminating the display if it's currently used by another `EglContext`.
+static DISPLAYS_REFERENCE_COUNT: Lazy<Mutex<HashMap<usize, usize>>> = Lazy::new(Default::default);
+
+fn initialize_display(
+    egl: &EglInstance,
+    display: khronos_egl::Display,
+) -> Result<(i32, i32), khronos_egl::Error> {
+    let mut guard = DISPLAYS_REFERENCE_COUNT.lock();
+    *guard.entry(display.as_ptr() as usize).or_default() += 1;
+
+    // We don't need to check the reference count here since according to the `eglInitialize`
+    // documentation, initializing an already initialized EGL display connection has no effect
+    // besides returning the version numbers.
+    egl.initialize(display)
+}
+
+fn terminate_display(
+    egl: &EglInstance,
+    display: khronos_egl::Display,
+) -> Result<(), khronos_egl::Error> {
+    let key = &(display.as_ptr() as usize);
+    let mut guard = DISPLAYS_REFERENCE_COUNT.lock();
+    let count_ref = guard
+        .get_mut(key)
+        .expect("Attempted to decref a display before incref was called");
+
+    if *count_ref > 1 {
+        *count_ref -= 1;
+
+        Ok(())
+    } else {
+        guard.remove(key);
+
+        egl.terminate(display)
+    }
+}
+
 impl Inner {
     fn create(
         flags: wgt::InstanceFlags,
         egl: Arc<EglInstance>,
         display: khronos_egl::Display,
         force_gles_minor_version: wgt::Gles3MinorVersion,
     ) -> Result<Self, crate::InstanceError> {
-        let version = egl.initialize(display).map_err(|e| {
+        let version = initialize_display(&egl, display).map_err(|e| {
             crate::InstanceError::with_source(
                 String::from("failed to initialize EGL display connection"),
                 e,
@@ -608,7 +648,8 @@ impl Drop for Inner {
         {
             log::warn!("Error in destroy_context: {:?}", e);
         }
-        if let Err(e) = self.egl.instance.terminate(self.egl.display) {
+
+        if let Err(e) = terminate_display(&self.egl.instance, self.egl.display) {
             log::warn!("Error in terminate: {:?}", e);
         }
     }
@@ -778,7 +819,7 @@ impl crate::Instance<super::Api> for Instance {
                 let display = unsafe {
                     egl.get_platform_display(
                         EGL_PLATFORM_SURFACELESS_MESA,
-                        std::ptr::null_mut(),
+                        khronos_egl::DEFAULT_DISPLAY,
                         &[khronos_egl::ATTRIB_NONE],
                     )
                 }