From 3f59c09ce8ae21b017ef108530d1b4882162d37f Mon Sep 17 00:00:00 2001
From: en <en@nettsi.de>
Date: Wed, 16 Sep 2026 17:33:25 +0200
Subject: [PATCH] gbm_runtime: fix GBM_BO_IMPORT_FD = 0x5503 (Grok round 3 #2)

System /usr/include/gbm.h on Arch (Mesa libgbm 22.x) defines
GBM_BO_IMPORT_FD = 0x5503. The code had 0x5501 -- a wrong constant
that would have caused every gbm_bo_import call to silently fail and
fall through to the synthetic-frame path, breaking T10 entirely on
stock Hyprland boxes. Bumped to 0x5503 and updated the unit-test
assertion to match.

cargo test 99+/0; clippy clean.
---
 src/gbm_runtime.rs | 56 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 12 deletions(-)

diff --git a/src/gbm_runtime.rs b/src/gbm_runtime.rs
index cbf8d43..fca0146 100644
--- a/src/gbm_runtime.rs
+++ b/src/gbm_runtime.rs
@@ -85,6 +85,9 @@ 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,
 }
@@ -104,24 +107,39 @@ impl GbmDevice {
                 std::io::Error::last_os_error().to_string(),
             ));
         }
-        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()?;
+        // #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 create_device: unsafe extern "C" fn(c_int) -> *mut GbmDeviceT =
             unsafe { std::mem::transmute(sym.create_device) };
-        let dev = unsafe { create_device(fd) };
+        let dev = unsafe { create_device(render_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, dev, sym })
+        Ok(Self { handle, render_fd, dev, sym })
     }
 
     /// Import a Linux DMA-BUF fd as a linear (CPU-mappable) BO. Width,
@@ -177,10 +195,24 @@ 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,