From e1a817bc06c5805c810ac85083366c13e426ee5d Mon Sep 17 00:00:00 2001
From: Ian Douglas Scott <idscott@system76.com>
Date: Mon, 12 May 2025 12:32:39 -0700
Subject: [PATCH] kms/surface: Fix thread crash on error return of `redraw`

If `redraw()` returned early, before updating `self.state`, but after
calling `queue_frame`, `on_vblank` would later be called, and reach
`unreachable` since state isn't set to `WaitinfForVBlank`.

In particular, this was happening when the dmabuf from the image copy
frame failed to `bind`.

To avoid this, make sure to update `self.state` immediately after
calling `queue_frame`, before any early return from an error.
---
 src/backend/kms/surface/mod.rs | 35 ++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/src/backend/kms/surface/mod.rs b/src/backend/kms/surface/mod.rs
index 3c33ac46..b0d39102 100644
--- a/src/backend/kms/surface/mod.rs
+++ b/src/backend/kms/surface/mod.rs
@@ -1431,6 +1431,25 @@ impl SurfaceThreadState {
                     x @ Ok(()) | x @ Err(FrameError::EmptyFrame) => {
                         self.timings.submitted_for_presentation(&self.clock);
 
+                        // Update `state` after `queue_frame`, before any early return from errors
+                        if x.is_ok() {
+                            let new_state = QueueState::WaitingForVBlank {
+                                redraw_needed: false,
+                            };
+                            match mem::replace(&mut self.state, new_state) {
+                                QueueState::Idle => unreachable!(),
+                                QueueState::Queued(_) => (),
+                                QueueState::WaitingForVBlank { .. } => unreachable!(),
+                                QueueState::WaitingForEstimatedVBlank(estimated_vblank)
+                                | QueueState::WaitingForEstimatedVBlankAndQueued {
+                                    estimated_vblank,
+                                    ..
+                                } => {
+                                    self.loop_handle.remove(estimated_vblank);
+                                }
+                            };
+                        }
+
                         for (session, frame, res) in frames {
                             let damage = match res {
                                 Ok((damage, _)) => damage,
@@ -1691,22 +1710,6 @@ impl SurfaceThreadState {
                         }
 
                         if x.is_ok() {
-                            let new_state = QueueState::WaitingForVBlank {
-                                redraw_needed: false,
-                            };
-                            match mem::replace(&mut self.state, new_state) {
-                                QueueState::Idle => unreachable!(),
-                                QueueState::Queued(_) => (),
-                                QueueState::WaitingForVBlank { .. } => unreachable!(),
-                                QueueState::WaitingForEstimatedVBlank(estimated_vblank)
-                                | QueueState::WaitingForEstimatedVBlankAndQueued {
-                                    estimated_vblank,
-                                    ..
-                                } => {
-                                    self.loop_handle.remove(estimated_vblank);
-                                }
-                            };
-
                             if self.mirroring.is_none() {
                                 self.frame_callback_seq = self.frame_callback_seq.wrapping_add(1);
                                 self.send_frame_callbacks();