search

gfx-rs / wgpu

A cross-platform, safe, pure-Rust graphics API.
https://wgpu.rs
Apache License 2.0
12.58k stars 920 forks source link

WGL and EGL's context activation is unsound #6211

Open ErichDonGubler opened 2 months ago

ErichDonGubler commented 2 months ago

Description

@MarijnS95 and I have determined that, in at least one case, the current WGL and EGL GLES backends are unsound, because conditions for safely conflict with the Soundness Property.

Repro steps

Will defer to @MarijnS95.

Expected vs observed behavior

These APIs should be sound, meaning that it is impossible for safe Rust to provoke UB or other memory unsafety.

MarijnS95 commented 1 month ago

There is no direct repro other than that the safety constraints at:

https://github.com/gfx-rs/wgpu/blob/e7c139b1d4c6b92987d833fddff7b1a30df6b126/wgpu-hal/src/gles/egl.rs#L1105-L1112

are invalid/impossible to uphold after the call to new_external(). I copied the same for WGL:

https://github.com/gfx-rs/wgpu/blob/e7c139b1d4c6b92987d833fddff7b1a30df6b126/wgpu-hal/src/gles/wgl.rs#L588-L595

I've started getting into a solution at https://github.com/gfx-rs/wgpu/issues/5505#issuecomment-2331149399 per a request from @jimblandy, and the simple verdict is that WGPU should probably move towards GL context sharing to solve all this trouble. That way WGPU can still share resources, while owning its own unique context that it can (un)current at any point in time without affecting external contexts.

Unfortunately context sharing can be finicky based on the platform (https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/merge_requests/1486) and might require a larger refactor of wgpu's gles module. glutin already wraps and provides all of this in higher-level and safe APIs, and should make this migration easier.

Separate from that there appear to be some hacks to replace the context on the fly as soon as a surface is available. This is entirely unavoidable if we could create a wgpu Adapter with a given RawDisplayHandle to ensure it's using the exact same "display backend connection" as a windowing system from which Surfaces will be created/used.