From e4ebd3aae5c243087940f48de5cf27c46a569525 Mon Sep 17 00:00:00 2001 From: Oleksiy Syvokon Date: Fri, 3 Apr 2026 22:31:03 +0300 Subject: [PATCH] Fix crash in WgpuAtlas when viewing a screen share (#53088) When atlas tiles are rapidly allocated and freed (e.g. watching a shared screen in Collab), a texture can become unreferenced and be removed while GPU uploads for it are still pending. On the next frame, `flush_uploads` indexes into the now-empty texture slot and panics: ``` thread 'main' panicked at crates/gpui_wgpu/src/wgpu_atlas.rs:231:40: texture must exist... #11 core::option::expect_failed #12 gpui_wgpu::wgpu_atlas::WgpuAtlas::before_frame #13 gpui_wgpu::wgpu_renderer::WgpuRenderer::draw ``` This change drains pending uploads for a texture when it becomes unreferenced in `remove`, and skips uploads for missing textures in `flush_uploads` as a safety net. Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed occasional crashes when viewing a screen share --- crates/gpui_wgpu/src/wgpu_atlas.rs | 82 +++++++++++++++++++++++++++++- 1 file changed, 81 insertions(+), 1 deletion(-) diff --git a/crates/gpui_wgpu/src/wgpu_atlas.rs b/crates/gpui_wgpu/src/wgpu_atlas.rs index 3eba5c533f80d727425cc87ae89b754afa8722b1..55f6edee21b9f2da02268c66c665c34d5b52066a 100644 --- a/crates/gpui_wgpu/src/wgpu_atlas.rs +++ b/crates/gpui_wgpu/src/wgpu_atlas.rs @@ -115,6 +115,8 @@ impl PlatformAtlas for WgpuAtlas { if let Some(mut texture) = texture_slot.take() { texture.decrement_ref_count(); if texture.is_unreferenced() { + lock.pending_uploads + .retain(|upload| upload.id != texture.id); lock.storage[id.kind] .free_list .push(texture.id.index as usize); @@ -228,7 +230,9 @@ impl WgpuAtlasState { fn flush_uploads(&mut self) { for upload in self.pending_uploads.drain(..) { - let texture = &self.storage[upload.id]; + let Some(texture) = self.storage.get(upload.id) else { + continue; + }; let bytes_per_pixel = texture.bytes_per_pixel(); self.queue.write_texture( @@ -286,6 +290,15 @@ impl ops::IndexMut for WgpuAtlasStorage { } } +impl WgpuAtlasStorage { + fn get(&self, id: AtlasTextureId) -> Option<&WgpuAtlasTexture> { + self[id.kind] + .textures + .get(id.index as usize) + .and_then(|t| t.as_ref()) + } +} + impl ops::Index for WgpuAtlasStorage { type Output = WgpuAtlasTexture; fn index(&self, id: AtlasTextureId) -> &Self::Output { @@ -341,3 +354,70 @@ impl WgpuAtlasTexture { self.live_atlas_keys == 0 } } + +#[cfg(all(test, not(target_family = "wasm")))] +mod tests { + use super::*; + use gpui::{ImageId, RenderImageParams}; + use pollster::block_on; + use std::sync::Arc; + + fn test_device_and_queue() -> anyhow::Result<(Arc, Arc)> { + block_on(async { + let instance = wgpu::Instance::new(wgpu::InstanceDescriptor { + backends: wgpu::Backends::all(), + flags: wgpu::InstanceFlags::default(), + backend_options: wgpu::BackendOptions::default(), + memory_budget_thresholds: wgpu::MemoryBudgetThresholds::default(), + display: None, + }); + let adapter = instance + .request_adapter(&wgpu::RequestAdapterOptions { + power_preference: wgpu::PowerPreference::LowPower, + compatible_surface: None, + force_fallback_adapter: false, + }) + .await + .map_err(|error| anyhow::anyhow!("failed to request adapter: {error}"))?; + let (device, queue) = adapter + .request_device(&wgpu::DeviceDescriptor { + label: Some("wgpu_atlas_test_device"), + required_features: wgpu::Features::empty(), + required_limits: wgpu::Limits::downlevel_defaults() + .using_resolution(adapter.limits()) + .using_alignment(adapter.limits()), + memory_hints: wgpu::MemoryHints::MemoryUsage, + trace: wgpu::Trace::Off, + experimental_features: wgpu::ExperimentalFeatures::disabled(), + }) + .await + .map_err(|error| anyhow::anyhow!("failed to request device: {error}"))?; + Ok((Arc::new(device), Arc::new(queue))) + }) + } + + #[test] + fn before_frame_skips_uploads_for_removed_texture() -> anyhow::Result<()> { + let (device, queue) = test_device_and_queue()?; + + let atlas = WgpuAtlas::new(device, queue); + let key = AtlasKey::Image(RenderImageParams { + image_id: ImageId(1), + frame_index: 0, + }); + let size = Size { + width: DevicePixels(1), + height: DevicePixels(1), + }; + let mut build = || Ok(Some((size, Cow::Owned(vec![0, 0, 0, 255])))); + + // Regression test: before the fix, this panicked in flush_uploads + atlas + .get_or_insert_with(&key, &mut build)? + .expect("tile should be created"); + atlas.remove(&key); + atlas.before_frame(); + + Ok(()) + } +}