From 0b2a139f98733810ee39ea65e74797ea13937afa Mon Sep 17 00:00:00 2001
From: Shawn Wallace <yungwallace@live.com>
Date: Sun, 29 Jun 2025 16:02:02 -0400
Subject: [PATCH] server: verify window still exists when processing surface
 serial

Should fix #191
---
 src/server/dispatch.rs | 20 ++++++++++-------
 src/server/mod.rs      |  4 ++--
 src/server/tests.rs    | 49 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 10 deletions(-)

diff --git a/src/server/dispatch.rs b/src/server/dispatch.rs
index f6dabfc..c5f24e3 100644
--- a/src/server/dispatch.rs
+++ b/src/server/dispatch.rs
@@ -1,5 +1,5 @@
 use super::*;
-use hecs::CommandBuffer;
+use hecs::{CommandBuffer, DynamicBundle};
 use log::{debug, error, trace, warn};
 use macros::simple_event_shunt;
 use std::sync::{Arc, OnceLock};
@@ -1435,13 +1435,19 @@ impl<C: XConnection> Dispatch<XwaylandSurfaceV1, Entity> for ServerState<C> {
                     .map(|i| i.0);
 
                 if let Some(win_entity) = win_entity {
-                    let win = *state.world.get::<&x::Window>(win_entity).unwrap();
-                    debug!("associate {surface_id} with {win:?}");
-                    state.windows.insert(win, *entity);
+                    let bundle = state.world.take(win_entity).unwrap();
+                    if !bundle.has::<x::Window>() {
+                        warn!("Window with same serial ({serial:?}) as {surface_id} has been destroyed?");
+                        return;
+                    }
                     let mut builder = hecs::EntityBuilder::new();
-                    builder.add_bundle(state.world.take(win_entity).unwrap());
+                    builder.add_bundle(bundle);
                     state.world.insert(*entity, builder.build()).unwrap();
+                    state.world.remove_one::<SurfaceSerial>(*entity).unwrap();
                     let data = state.world.entity(*entity).unwrap();
+                    let win = data.get::<&x::Window>().as_deref().copied().unwrap();
+                    state.windows.insert(win, *entity);
+                    debug!("associate {surface_id} with {win:?} (serial {serial:?})");
                     if data.get::<&WindowData>().unwrap().mapped {
                         state.create_role_window(win, *entity);
                     }
@@ -1449,9 +1455,7 @@ impl<C: XConnection> Dispatch<XwaylandSurfaceV1, Entity> for ServerState<C> {
                     state.world.insert(*entity, (serial,)).unwrap();
                 }
             }
-            Request::Destroy => {
-                state.world.remove_one::<SurfaceSerial>(*entity).unwrap();
-            }
+            Request::Destroy => {}
             _ => unreachable!(),
         }
     }
diff --git a/src/server/mod.rs b/src/server/mod.rs
index 6b8c165..7d35cf9 100644
--- a/src/server/mod.rs
+++ b/src/server/mod.rs
@@ -172,7 +172,7 @@ struct SurfaceAttach {
     y: i32,
 }
 
-#[derive(PartialEq, Eq)]
+#[derive(PartialEq, Eq, Debug)]
 struct SurfaceSerial([u32; 2]);
 
 #[derive(Debug)]
@@ -749,7 +749,7 @@ impl<C: XConnection> ServerState<C> {
         if dims == win.attrs.dims {
             return;
         }
-        debug!("Reconfiguring {win:?} {:?}", dims);
+        debug!("Reconfiguring {:?} {:?}", event.window(), dims);
         if !win.mapped {
             win.attrs.dims = dims;
             return;
diff --git a/src/server/tests.rs b/src/server/tests.rs
index 786dfc2..b6728ca 100644
--- a/src/server/tests.rs
+++ b/src/server/tests.rs
@@ -2444,6 +2444,55 @@ fn quick_empty_data_offer() {
     let selection = f.satellite.new_selection();
     assert!(selection.is_none());
 }
+
+#[test]
+fn quick_destroy_window_with_serial() {
+    let (mut f, comp) = TestFixture::new_with_compositor();
+
+    let window = unsafe { Window::new(1) };
+    let data = WindowData {
+        mapped: true,
+        dims: WindowDims {
+            x: 0,
+            y: 0,
+            width: 50,
+            height: 50,
+        },
+        fullscreen: false,
+    };
+    f.new_window(window, false, data);
+    f.satellite.map_window(window);
+
+    let (_, surface) = comp.create_surface();
+    let xwl = TestObject::<XwaylandSurfaceV1>::from_request(
+        &comp.shell.obj,
+        Req::<XwaylandShellV1>::GetXwaylandSurface {
+            surface: surface.clone(),
+        },
+    );
+
+    let serial = f.surface_serial;
+    let serial_lo = (serial & 0xFF) as u32;
+    let serial_hi = ((serial >> 8) & 0xFF) as u32;
+    xwl.send_request(Req::<XwaylandSurfaceV1>::SetSerial {
+        serial_lo,
+        serial_hi,
+    })
+    .unwrap();
+    f.satellite
+        .set_window_serial(window, [serial_lo, serial_hi]);
+    f.satellite.unmap_window(window);
+    f.satellite.destroy_window(window);
+    f.run();
+
+    let id = f.testwl.last_created_surface_id().unwrap();
+    let surface_data = f.testwl.get_surface_data(id).unwrap();
+    assert!(
+        surface_data.role.is_none(),
+        "Surface unexpectedly has role: {:?}",
+        surface_data.role
+    );
+}
 /// See Pointer::handle_event for an explanation.
 #[test]
 fn popup_pointer_motion_workaround() {}