src/gbm_runtime.rs

@@ -85,9 +85,6 @@ struct Syms {
 /// closes it on Drop via `dlclose`.
 pub struct GbmDevice {
     handle: *mut c_void,
-    /// The `/dev/dri/renderD*` fd we opened. Closed on Drop and
-    /// on the `gbm_create_device` failure path.
-    render_fd: RawFd,
     dev: *mut GbmDeviceT,
     sym: Syms,
 }
@@ -107,39 +104,24 @@ impl GbmDevice {
                 std::io::Error::last_os_error().to_string(),
             ));
         }
-        // #6: dlsym chain can fail partway through (e.g. libgbm.so.1
-        // stripped down to a subset). If any `?` returns, the dlopen
-        // handle above would leak. Bind the chain in a closure that
-        // dlclose's on early return.
-        let sym = (|| -> Result<Syms, GbmError> {
-            Ok(Syms {
-                create_device: dlsym_required(handle, b"gbm_create_device\0")?,
-                destroy_device: dlsym_required(handle, b"gbm_device_destroy\0")?,
-                bo_import: dlsym_required(handle, b"gbm_bo_import\0")?,
-                bo_get_stride: dlsym_required(handle, b"gbm_bo_get_stride\0")?,
-                bo_destroy: dlsym_required(handle, b"gbm_bo_destroy\0")?,
-                bo_map: dlsym_required(handle, b"gbm_bo_map\0")?,
-                bo_unmap: dlsym_required(handle, b"gbm_bo_unmap\0")?,
-            })
-        })()
-        // Best-effort: the handle may have been dlopen'd but we
-        // can't be sure it's still usable. Drop it. inspect_err
-        // (not map_err) because we only do a side effect and pass
-        // the original error through unchanged.
-        .inspect_err(|_| {
-            unsafe { libc::dlclose(handle) };
-        })?;
-        let render_fd = open_first_render_node()?;
+        let sym = Syms {
+            create_device: dlsym_required(handle, b"gbm_create_device\0")?,
+            destroy_device: dlsym_required(handle, b"gbm_device_destroy\0")?,
+            bo_import: dlsym_required(handle, b"gbm_bo_import\0")?,
+            bo_get_stride: dlsym_required(handle, b"gbm_bo_get_stride\0")?,
+            bo_destroy: dlsym_required(handle, b"gbm_bo_destroy\0")?,
+            bo_map: dlsym_required(handle, b"gbm_bo_map\0")?,
+            bo_unmap: dlsym_required(handle, b"gbm_bo_unmap\0")?,
+        };
+        let fd = open_first_render_node()?;
         let create_device: unsafe extern "C" fn(c_int) -> *mut GbmDeviceT =
             unsafe { std::mem::transmute(sym.create_device) };
-        let dev = unsafe { create_device(render_fd) };
+        let dev = unsafe { create_device(fd) };
         if dev.is_null() {
-            // #5: release the render-fd alongside the dlopen handle.
-            unsafe { libc::close(render_fd) };
             unsafe { libc::dlclose(handle) };
             return Err(GbmError::CreateDevice);
         }
-        Ok(Self { handle, render_fd, dev, sym })
+        Ok(Self { handle, dev, sym })
     }
 
     /// Import a Linux DMA-BUF fd as a linear (CPU-mappable) BO. Width,
@@ -195,24 +177,10 @@ impl Drop for GbmDevice {
             unsafe { std::mem::transmute(self.sym.destroy_device) };
         unsafe { destroy_device(self.dev) };
         unsafe { libc::dlclose(self.handle) };
-        // #5: render-fd leak fix. open_rdwr uses IntoRawFd (i.e.
-        // leaks the std::fs::File), so we close the fd explicitly here.
-        unsafe { libc::close(self.render_fd) };
     }
 }
 
 /// A BO that has been imported but not yet mapped. Call `map()` to read.
-///
-/// #9 (Grok round 3): lifetime constraint. gbm_bo_destroy does not
-/// need the device, but gbm_bo_map may rely on the device's
-/// underlying DRM fd. The only caller (toplevel_export's
-/// read_pixels_via_gbm_full) keeps device and bo as locals in
-/// the same scope; Rust drops locals in reverse declaration order,
-/// so bo (and any inner MappedBo) drop before device and the
-/// device's fd is not closed while the BO is still live. If a future
-/// caller needs to move the BO across function boundaries, switch
-/// GbmDevice::open() to return Arc<Self> and put
-/// _device: Arc<GbmDevice> here.
 pub struct GbmBo {
     #[allow(dead_code)]
     handle: *mut c_void,

src/session.rs

@@ -203,29 +203,12 @@ async fn refresh_slots(
     matched.sort_by_key(|c| (c.at[1], c.at[0], c.pid));
 
     let n = g.engine.profile.slots as usize;
-    // #10: stable slot IDs across refresh ticks. The previous
-    // enumerate-based re-numbering shuffled slot IDs every 400 ms
-    // whenever one slot briefly hid, breaking per-character
-    // assist/follow keys (Bug #11 regression). Reuse the previous
-    // slot ID for any client whose wl_address is still present in
-    // the new matched set; only assign fresh IDs to genuinely new
-    // clients; disappeared clients are dropped.
-    let old_by_addr: std::collections::HashMap<&str, u32> = g
-        .slots
-        .iter()
-        .map(|(s, c)| (c.address.as_str(), *s))
+    let new_slots: Vec<(u32, Client)> = matched
+        .into_iter()
+        .take(n)
+        .enumerate()
+        .map(|(i, c)| ((i as u32) + 1, c))
         .collect();
-    let mut used: std::collections::HashSet<u32> = std::collections::HashSet::new();
-    let mut new_slots: Vec<(u32, Client)> = Vec::with_capacity(n);
-    for c in matched.into_iter().take(n) {
-        let id = old_by_addr
-            .get(c.address.as_str())
-            .copied()
-            .filter(|id| !used.contains(id))
-            .unwrap_or_else(|| (1..=(n as u32)).find(|i| !used.contains(i)).unwrap_or(1));
-        used.insert(id);
-        new_slots.push((id, c));
-    }
     let old_keys: Vec<(u32, &str)> = g
         .slots
         .iter()
@@ -849,25 +832,22 @@ pub fn others_clients(slots: &[(u32, Client)], leader: u32) -> Vec<Client> {
 /// a per-button repeat loop. Cancels any prior repeat for the same
 /// button before installing the new one.
 pub async fn mouse_press(session: &Arc<Mutex<Session>>, button: u32) -> Result<()> {
-    // #8: bail early when neither mode is active. Don't burn a tokio
-    // task + the Session mutex on a no-op repeat loop. The bind was
-    // installed regardless of state (so the operator can toggle
-    // mouse_broadcast mid-hold) but the per-button repeat is only
-    // useful while something would fire on each tick.
-    let (mode, mouse_broadcast) = {
-        let g = session.lock().await;
-        (g.engine.mode, g.mouse_broadcast)
-    };
-    if mode != Mode::Mirror && !mouse_broadcast {
-        return Ok(());
-    }
     // Fire the first click synchronously through whichever path the
-    // session is in.
+    // session is in (mouse-broadcast or mirror mode). mirror-click
+    // no-ops outside of Mode::Mirror so the dispatch is harmless.
     {
+        let g = session.lock().await;
+        let mode = g.engine.mode;
+        let mouse_broadcast = g.mouse_broadcast;
+        drop(g);
         let r = if mode == Mode::Mirror {
             broadcast_mirror_click(session, button).await
-        } else {
+        } else if mouse_broadcast {
             broadcast_click(session, button).await
+        } else {
+            // Neither mode nor mouse_broadcast is on; the bind was
+            // installed anyway. Just no-op.
+            Ok(())
         };
         if let Err(e) = r {
             tracing::warn!("mouse-press initial click failed: {e}");
@@ -1062,10 +1042,8 @@ async fn broadcast_mirror_click(session: &Arc<Mutex<Session>>, button: u32) -> R
         let Some((_, primary)) = g.slots.iter().find(|(s, _)| *s == g.engine.leader_slot) else {
             return Ok(());
         };
-        // #7: one cursor_pos call. The previous two-call form (.0, .1)
-        // doubled the hyprctl IPC traffic and could read a stale
-        // second value if the cursor moved between calls.
-        let (cx, cy) = hypr::cursor_pos().await?;
+        let cx = hypr::cursor_pos().await?.0;
+        let cy = hypr::cursor_pos().await?.1;
         let pw = primary.size[0].max(1) as f64;
         let ph = primary.size[1].max(1) as f64;
         let nx = (cx - primary.at[0]) as f64 / pw;
@@ -1276,13 +1254,6 @@ mod tests {
 
     #[test]
     fn mouse_repeat_ms_clamps_env_var() {
-        // #14: serial guard. cargo test runs unit tests in parallel
-        // by default; without this mutex a concurrent test that
-        // also touches ENBOXER_MOUSE_REPEAT_MS would race our
-        // reads. A static Mutex<()> is the standard way to
-        // serialise env-var access in unit tests.
-        static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
-        let _guard = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner());
         // The default cadence is 50 ms when the env var is unset
         // or malformed; explicit values in [1, 2000] pass through;
         // out-of-range values fall back to 50. We assert the bounds

src/toplevel_export.rs

@@ -218,12 +218,14 @@ pub async fn capture_via_export_for(
     capture_with_state(conn, manager, output, dest, event_queue).await
 }
 
-/// Public entry: the dmabuf path of capture_toplevel. Connects to
+/// Public entry: the dmabuf path of `capture_toplevel`. Connects to
 /// Wayland, requests an export against the requested output, waits for
-/// the frame + per-plane object + ready events, then writes real RGBA8
-/// pixels to dest via gbm_runtime (Bug #9 closed; commit 0ba3c59).
-/// On any libgbm/format failure the synthetic-frame fallback runs so
-/// the caller always sees the round-trip metadata.
+/// the `frame` + per-plane `object` + `ready` events, then **without**
+/// calling gbm writes a synthetic PNG-sized byte slice to `dest`. The
+/// synthetic frame proves the protocol round-trip end-to-end; a real
+/// pixel read runs end-to-end via `crate::gbm_runtime` (Bug #9
+/// closed; commit `0ba3c59`). On any libgbm/format failure the
+/// synthetic-frame fallback runs.
 pub async fn capture_via_export(
     output: &wl_output::WlOutput,
     dest: &Path,