GPU: no texture of a failed frame returns to the pool, even one out or waiting on another encoder (refs #1319)

crates/gpu/src/context.rs

@@ -73,6 +73,8 @@ struct Lease {
     pool: Weak<Mutex<Pool>>,
     /// The [`Enc`] that took it (see [`Pool::owned`]).
     owner: u64,
+    /// The pool's [`Pool::generation`] when it was taken.
+    generation: u64,
 }
 
 impl std::fmt::Debug for Lease {
@@ -86,7 +88,7 @@ impl Drop for Lease {
         if let Some(pool) = self.pool.upgrade()
             && let Ok(mut p) = pool.lock()
         {
-            p.release_owned(self.texture.clone(), self.owner);
+            p.release_owned(self.texture.clone(), self.owner, self.generation);
         }
     }
 }
@@ -117,6 +119,8 @@ pub(crate) struct Pool {
     /// working texture of another `Enc` before it is gone.
     owned: HashMap<u64, HashMap<(u32, u32), Vec<wgpu::Texture>>>,
     next_owner: u64,
+    /// Raised by [`Pool::discard`]: a texture taken before it never comes back to the pool.
+    generation: u64,
 }
 
 /// In-`Enc` texture reuse ([`Pool::owned`]); tests turn it off to compare.
@@ -207,7 +211,10 @@ impl Pool {
         }
     }
 
-    fn release_owned(&mut self, t: wgpu::Texture, owner: u64) {
+    fn release_owned(&mut self, t: wgpu::Texture, owner: u64, generation: u64) {
+        if generation != self.generation {
+            return;
+        }
         if let Some(m) = self.owned.get_mut(&owner) {
             m.entry((t.width(), t.height())).or_default().push(t);
             return;
@@ -237,6 +244,18 @@ impl Pool {
         self.free.clear();
         self.bytes = 0;
     }
+
+    /// After a failed frame (#1319): drop every kept texture, the free ones and those waiting
+    /// for an open encoder or kept by one, and the textures still out never return. The
+    /// encoders' bookkeeping stays, so reuse remains ordered.
+    fn discard(&mut self) {
+        self.clear();
+        self.released.clear();
+        for kept in self.owned.values_mut() {
+            kept.clear();
+        }
+        self.generation = self.generation.wrapping_add(1);
+    }
 }
 
 /// Uniform parameter block shared by every kernel (see `shaders/common.wgsl`).
@@ -867,10 +886,10 @@ impl GpuContext {
     /// one is zeroed.
     fn pooled(&self, w: u32, h: u32, owner: u64) -> (GpuImage, bool) {
         let (w, h) = (w.max(1), h.max(1));
-        let reused = self.pool.lock().ok().and_then(|mut p| p.take_owned(owner, w, h).or_else(|| p.take(w, h)));
+        let (reused, generation) = self.pool.lock().map(|mut p| (p.take_owned(owner, w, h).or_else(|| p.take(w, h)), p.generation)).unwrap_or((None, u64::MAX));
         let was_reused = reused.is_some();
         let texture = reused.unwrap_or_else(|| self.texture(w, h));
-        let lease = Arc::new(Lease { texture: texture.clone(), pool: Arc::downgrade(&self.pool), owner });
+        let lease = Arc::new(Lease { texture: texture.clone(), pool: Arc::downgrade(&self.pool), owner, generation });
         (GpuImage { texture, width: w, height: h, _lease: Some(lease), levels: None }, was_reused)
     }
 
@@ -985,6 +1004,9 @@ impl GpuContext {
     /// may be one the device rejected (#1319). Later frames allocate afresh.
     pub fn discard_kept(&self) {
         self.clear_uploads();
+        if let Ok(mut p) = self.pool.lock() {
+            p.discard();
+        }
         if let Ok(mut z) = self.zeros.lock() {
             z.clear();
         }

crates/gpu/src/tests.rs

@@ -780,6 +780,49 @@ fn a_rejected_texture_fails_the_frame_not_the_compositor() {
     assert_eq!(g.ctx.pool_stats().0, hits, "the failed frame's texture wasn't reused");
 }
 
+/// A texture the device rejected (deliberately too wide), made outside any frame under a scope
+/// of its own: the view a frame then makes of it is the reported validation error.
+fn rejected_texture(g: &Gpu) -> wgpu::Texture {
+    let scope = g.ctx.device.push_error_scope(wgpu::ErrorFilter::Validation);
+    let too_wide = g.ctx.device.limits().max_texture_dimension_2d.saturating_add(1);
+    let t = g.ctx.device.create_texture(&wgpu::TextureDescriptor {
+        label: Some("deliberately too wide"),
+        size: wgpu::Extent3d { width: too_wide, height: 1, depth_or_array_layers: 1 },
+        mip_level_count: 1,
+        sample_count: 1,
+        dimension: wgpu::TextureDimension::D2,
+        format: wgpu::TextureFormat::Rgba8Unorm,
+        usage: wgpu::TextureUsages::TEXTURE_BINDING,
+        view_formats: &[],
+    });
+    assert!(pollster::block_on(scope.pop()).is_some(), "the device rejected the texture");
+    t
+}
+
+/// No texture of a failed frame is reused: not one released while another encoder is still
+/// open (it waits for that encoder), nor one taken before the failure and handed back after.
+#[test]
+fn a_failed_frame_returns_no_texture_to_the_pool() {
+    hold_gpu_lock();
+    let Some(g) = Gpu::headless() else { return };
+    let rejected = rejected_texture(&g);
+    let earlier = g.within_memory(|| crate::context::Enc::new(&g.ctx).image(48, 16)).expect("an earlier frame");
+    let mut other = crate::context::Enc::new(&g.ctx);
+    other.encoder();
+    g.within_memory(|| (crate::context::Enc::new(&g.ctx).image(64, 32), rejected.create_view(&Default::default()))).expect_err("the frame fails");
+    drop(earlier);
+    drop(other);
+    let (hits, _) = g.ctx.pool_stats();
+    let (a, b) = g
+        .within_memory(|| {
+            let mut e = crate::context::Enc::new(&g.ctx);
+            (e.image(64, 32), e.image(48, 16))
+        })
+        .expect("the next frame");
+    assert_eq!(((a.width, a.height), (b.width, b.height)), ((64, 32), (48, 16)));
+    assert_eq!(g.ctx.pool_stats().0, hits, "no texture of the failed frame was reused");
+}
+
 /// A buffer whose allocation fails (here: over the device's buffer size limit; on Adreno an
 /// internal `ERROR_INITIALIZATION_FAILED`) is reported to the caller's error scope instead of
 /// panicking in `create_buffer_init`'s mapped range on a frame thread (#591).