Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Mar 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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!!!**

Expand Down Expand Up @@ -164,6 +164,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

Expand Down
86 changes: 58 additions & 28 deletions tests/tests/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
49 changes: 45 additions & 4 deletions wgpu-hal/src/gles/egl.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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],
)
}
Expand Down
Loading