Explain runtime invariants and avoid redundant unwraps

compositor/src/handlers/layer_shell.rs

@@ -177,7 +177,9 @@ pub fn handle_commit(state: &mut Ferese, surface: &WlSurface) {
         std::sync::Mutex::new(None::<(LayoutState, KeyboardInteractivity)>)
     });
     let (changed, requested_focus) = {
-        let mut previous = previous.lock().unwrap();
+        let mut previous = previous
+            .lock()
+            .expect("layer configure state must remain consistent across callbacks");
         let changed = previous.as_ref().map(|(layout, _)| layout) != Some(&current);
         let requested_focus = policy.keyboard_interactivity == KeyboardInteractivity::OnDemand
             && previous

compositor/src/handlers/screencopy.rs

@@ -250,7 +250,10 @@ impl Dispatch<ZwlrScreencopyFrameV1, Arc<FrameData>> for Ferese {
                 state.queue_screencopy(frame, data, buffer, true);
             }
             zwlr_screencopy_frame_v1::Request::Destroy => {
-                data.snapshot.lock().unwrap().take();
+                data.snapshot
+                    .lock()
+                    .expect("surface rendering state must remain consistent across callbacks")
+                    .take();
             }
             _ => unreachable!(),
         }
@@ -265,11 +268,19 @@ impl Ferese {
         buffer: WlBuffer,
         with_damage: bool,
     ) {
-        if data.snapshot.lock().unwrap().is_some() {
+        if data
+            .snapshot
+            .lock()
+            .expect("surface rendering state must remain consistent across callbacks")
+            .is_some()
+        {
             super::window_capture::copy_snapshot(self, frame, data, &buffer);
             return;
         }
-        let mut used = data.used.lock().unwrap();
+        let mut used = data
+            .used
+            .lock()
+            .expect("surface rendering state must remain consistent across callbacks");
         if self.session_lock.active() {
             frame.failed();
             return;

compositor/src/handlers/window_capture.rs

@@ -35,7 +35,9 @@ struct Permit {
 
 impl Permit {
     fn reserve(budget: &Arc<Mutex<Budget>>, bytes: usize) -> Option<Self> {
-        let mut available = budget.lock().unwrap();
+        let mut available = budget
+            .lock()
+            .expect("surface rendering state must remain consistent across callbacks");
         if bytes == 0
             || bytes > MAX_BYTES.saturating_sub(available.bytes)
             || available.frames >= MAX_FRAMES
@@ -53,7 +55,10 @@ impl Permit {
 
 impl Drop for Permit {
     fn drop(&mut self) {
-        let mut budget = self.budget.lock().unwrap();
+        let mut budget = self
+            .budget
+            .lock()
+            .expect("surface rendering state must remain consistent across callbacks");
         budget.bytes -= self.bytes;
         budget.frames -= 1;
     }
@@ -167,21 +172,32 @@ fn finish_conversion(state: &Ferese, result: Conversion) {
     let Some(data) = frame.data::<Arc<FrameData>>() else {
         return;
     };
-    if !frame.is_alive() || *data.used.lock().unwrap() {
+    if !frame.is_alive()
+        || *data
+            .used
+            .lock()
+            .expect("surface rendering state must remain consistent across callbacks")
+    {
         return;
     }
     if state.session_lock.active()
         || !super::screencopy::capture_allowed()
         || !state.capture_window_allowed(snapshot.id, snapshot.epoch)
     {
-        *data.used.lock().unwrap() = true;
+        *data
+            .used
+            .lock()
+            .expect("surface rendering state must remain consistent across callbacks") = true;
         frame.failed();
         return;
     }
 
     let size = (snapshot.pixels.width as u32, snapshot.pixels.height as u32);
     let logical_size = snapshot.logical_size;
-    *data.snapshot.lock().unwrap() = Some(snapshot);
+    *data
+        .snapshot
+        .lock()
+        .expect("surface rendering state must remain consistent across callbacks") = Some(snapshot);
     if manager.is_alive() {
         manager.geometry(&frame, logical_size.0, logical_size.1);
     }
@@ -256,7 +272,8 @@ impl Dispatch<FereseWindowCaptureManagerV1, Global> for Ferese {
             } else if global
                 .worker
                 .try_send(Conversion {
-                    snapshot: snapshot.unwrap(),
+                    snapshot: snapshot
+                        .expect("conversion is queued only after obtaining a capture snapshot"),
                     frame: resource.clone(),
                     manager: manager.clone(),
                 })
@@ -264,10 +281,11 @@ impl Dispatch<FereseWindowCaptureManagerV1, Global> for Ferese {
             {
                 *resource
                     .data::<Arc<FrameData>>()
-                    .unwrap()
+                    .expect("capture frame resources are initialized with their FrameData")
                     .used
                     .lock()
-                    .unwrap() = true;
+                    .expect("surface rendering state must remain consistent across callbacks") =
+                    true;
                 manager.busy(&resource);
                 resource.failed();
             }
@@ -386,7 +404,10 @@ pub(super) fn copy_snapshot(
     data: &FrameData,
     buffer: &WlBuffer,
 ) {
-    let mut used = data.used.lock().unwrap();
+    let mut used = data
+        .used
+        .lock()
+        .expect("surface rendering state must remain consistent across callbacks");
     if *used {
         resource.post_error(
             frame::Error::AlreadyUsed,
@@ -395,7 +416,12 @@ pub(super) fn copy_snapshot(
         return;
     }
     *used = true;
-    let Some(snapshot) = data.snapshot.lock().unwrap().take() else {
+    let Some(snapshot) = data
+        .snapshot
+        .lock()
+        .expect("surface rendering state must remain consistent across callbacks")
+        .take()
+    else {
         resource.failed();
         return;
     };

compositor/src/handlers/xdg_shell.rs

@@ -329,9 +329,9 @@ pub(crate) fn initial_configure_sent(toplevel: &ToplevelSurface) -> bool {
         states
             .data_map
             .get::<XdgToplevelSurfaceData>()
-            .unwrap()
+            .expect("an XDG toplevel must retain its Smithay surface data")
             .lock()
-            .unwrap()
+            .expect("Smithay surface state must remain consistent across callbacks")
             .initial_configure_sent
     })
 }