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

gfx-rs · Mar 6, 2024 · 3920581 · 3920581
1 parent badb3c8
commit 3920581
Show file tree

Hide file tree

Showing 3 changed files with 95 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!!!**
 
@@ -160,6 +160,7 @@ By @cwfitzgerald in [#5325](https://github.com/gfx-rs/wgpu/pull/5325).
 #### GLES
 
 - 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).
+- Fix crash when holding multiple devices on wayland/surfaceless. By @ashdnazg in [#5351](https://github.com/gfx-rs/wgpu/pull/5351).
 
 ## v0.19.0 (2024-01-17)
 

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,13 +433,40 @@ struct Inner {
     srgb_kind: SrgbFrameBufferKind,
 }
 
+static DISPLAYS_REFERENCE_COUNT: Lazy<Mutex<HashMap<usize, usize>>> = Lazy::new(Default::default);
+
+fn increment_display_reference_count(display: khronos_egl::Display) {
+    *DISPLAYS_REFERENCE_COUNT
+        .lock()
+        .entry(display.as_ptr() as usize)
+        .or_default() += 1;
+}
+
+fn decrement_display_reference_count(display: khronos_egl::Display) -> bool {
+    let key = &(display.as_ptr() as usize);
+    let mut guard = DISPLAYS_REFERENCE_COUNT.lock();
+    let count_ref = guard.get_mut(key).unwrap();
+
+    if *count_ref > 1 {
+        *count_ref -= 1;
+
+        false
+    } else {
+        guard.remove(key);
+
+        true
+    }
+}
+
 impl Inner {
     fn create(
         flags: wgt::InstanceFlags,
         egl: Arc<EglInstance>,
         display: khronos_egl::Display,
         force_gles_minor_version: wgt::Gles3MinorVersion,
     ) -> Result<Self, crate::InstanceError> {
+        increment_display_reference_count(display);
+
         let version = egl.initialize(display).map_err(|e| {
             crate::InstanceError::with_source(
                 String::from("failed to initialize EGL display connection"),
@@ -608,8 +636,11 @@ impl Drop for Inner {
         {
             log::warn!("Error in destroy_context: {:?}", e);
         }
-        if let Err(e) = self.egl.instance.terminate(self.egl.display) {
-            log::warn!("Error in terminate: {:?}", e);
+
+        if decrement_display_reference_count(self.egl.display) {
+            if let Err(e) = self.egl.instance.terminate(self.egl.display) {
+                log::warn!("Error in terminate: {:?}", e);
+            }
         }
     }
 }
@@ -778,7 +809,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],
                     )
                 }