fix(linux): close the DRM render-node fd owned by wl::dmabuf_t

src/platform/linux/wayland.cpp

@@ -42,8 +42,55 @@ namespace wl {
       .get_offset = gbm_bo_get_offset,
       .get_modifier = gbm_bo_get_modifier,
     };
+
+    int open_render_node(const char *path) {
+      return open(path, O_RDWR | O_CLOEXEC);
+    }
+
+    const gbm_device_accessors_t gbm_device_accessors {
+      .open_render_node = open_render_node,
+      .create_device = gbm_create_device,
+      .destroy_device = gbm_device_destroy,
+    };
   }  // namespace
 
+  gbm_device_t::~gbm_device_t() {
+    reset();
+  }
+
+  bool gbm_device_t::init(const std::string &render_path, const gbm_device_accessors_t &accessors) {
+    reset();
+    this->accessors = accessors;
+
+    drm_fd = accessors.open_render_node(render_path.c_str());
+    if (drm_fd < 0) {
+      BOOST_LOG(error) << "[wayland] Failed to open DRM render node: "sv << render_path;
+      return false;
+    }
+
+    device = accessors.create_device(drm_fd);
+    if (!device) {
+      BOOST_LOG(error) << "[wayland] Failed to create GBM device"sv;
+      reset();
+      return false;
+    }
+
+    return true;
+  }
+
+  void gbm_device_t::reset() {
+    if (device) {
+      accessors.destroy_device(device);
+      device = nullptr;
+    }
+
+    // gbm_device_destroy() does not close the descriptor the device was created from
+    if (drm_fd >= 0) {
+      close(drm_fd);
+      drm_fd = -1;
+    }
+  }
+
   // Helper to call C++ method from wayland C callback
   template<class T, class Method, Method m, class... Params>
   static auto classCall(void *data, Params... params) -> decltype(((*reinterpret_cast<T *>(data)).*m)(params...)) {
@@ -258,25 +305,11 @@ namespace wl {
 
   // Initialize GBM
   bool dmabuf_t::init_gbm() {
-    if (gbm_device) {
+    if (gbm) {
       return true;
     }
 
-    auto render_path = platf::resolve_render_device();
-    int drm_fd = open(render_path.c_str(), O_RDWR);
-    if (drm_fd < 0) {
-      BOOST_LOG(error) << "[wayland] Failed to open DRM render node: "sv << render_path;
-      return false;
-    }
-
-    gbm_device = gbm_create_device(drm_fd);
-    if (!gbm_device) {
-      close(drm_fd);
-      BOOST_LOG(error) << "[wayland] Failed to create GBM device"sv;
-      return false;
-    }
-
-    return true;
+    return gbm.init(platf::resolve_render_device(), gbm_device_accessors);
   }
 
   // Cleanup GBM
@@ -344,11 +377,7 @@ namespace wl {
       frame.destroy();
     }
 
-    if (gbm_device) {
-      // We should close the DRM FD, but it's owned by GBM
-      gbm_device_destroy(gbm_device);
-      gbm_device = nullptr;
-    }
+    gbm.reset();
   }
 
   // Buffer format callback
@@ -423,12 +452,12 @@ namespace wl {
     if (supported_modifiers) {
       auto it = supported_modifiers->find(dmabuf_info.format);
       if (it != supported_modifiers->end() && !it->second.empty()) {
-        current_bo = gbm_bo_create_with_modifiers2(gbm_device, dmabuf_info.width, dmabuf_info.height, dmabuf_info.format, it->second.data(), it->second.size(), GBM_BO_USE_RENDERING);
+        current_bo = gbm_bo_create_with_modifiers2(gbm.get(), dmabuf_info.width, dmabuf_info.height, dmabuf_info.format, it->second.data(), it->second.size(), GBM_BO_USE_RENDERING);
       }
     }
 
     if (!current_bo) {
-      current_bo = gbm_bo_create(gbm_device, dmabuf_info.width, dmabuf_info.height, dmabuf_info.format, GBM_BO_USE_RENDERING);
+      current_bo = gbm_bo_create(gbm.get(), dmabuf_info.width, dmabuf_info.height, dmabuf_info.format, GBM_BO_USE_RENDERING);
     }
 
     if (!current_bo) {

src/platform/linux/wayland.h

@@ -10,6 +10,7 @@
 #include <cstdint>
 #include <map>
 #include <optional>
+#include <string>
 #include <vector>
 
 #ifdef SUNSHINE_BUILD_WAYLAND
@@ -28,6 +29,7 @@
 #ifdef SUNSHINE_BUILD_WAYLAND
 
 struct gbm_bo;
+struct gbm_device;
 
 namespace wl {
   /**
@@ -54,6 +56,84 @@ namespace wl {
     std::uint64_t (*get_modifier)(gbm_bo *bo);  ///< Return the DRM format modifier shared by the planes.
   };
 
+  /**
+   * @brief Functions used to open a render node and create/destroy a GBM device on it.
+   */
+  struct gbm_device_accessors_t {
+    int (*open_render_node)(const char *path);  ///< Open the render node and return its descriptor, or -1.
+    gbm_device *(*create_device)(int fd);  ///< Create a GBM device on the descriptor, or nullptr.
+    void (*destroy_device)(gbm_device *device);  ///< Destroy a GBM device (does not close the descriptor).
+  };
+
+  /**
+   * @brief Owner of a GBM device together with the render-node descriptor it was created from.
+   *
+   * `gbm_device_destroy()` does not close the descriptor passed to `gbm_create_device()`,
+   * so the descriptor has to be tracked and closed here once the device is gone.
+   */
+  class gbm_device_t {
+  public:
+    /**
+     * @brief Construct an empty owner that holds neither a device nor a descriptor.
+     */
+    gbm_device_t() = default;
+
+    /**
+     * @brief Copying is disabled: the descriptor and the device have exactly one owner.
+     */
+    gbm_device_t(const gbm_device_t &) = delete;
+
+    /**
+     * @brief Copy assignment is disabled: the descriptor and the device have exactly one owner.
+     */
+    gbm_device_t &operator=(const gbm_device_t &) = delete;
+
+    /**
+     * @brief Destroy the GBM device and close the render-node descriptor.
+     */
+    ~gbm_device_t();
+
+    /**
+     * @brief Open the render node and create the GBM device on it.
+     *
+     * @param render_path Path of the DRM render node.
+     * @param accessors Functions used to open the node and create the device.
+     * @return `true` when the device is ready, `false` when nothing is held.
+     */
+    bool init(const std::string &render_path, const gbm_device_accessors_t &accessors);
+
+    /**
+     * @brief Destroy the GBM device and close the render-node descriptor.
+     */
+    void reset();
+
+    /**
+     * @return The GBM device, or nullptr.
+     */
+    gbm_device *get() const {
+      return device;
+    }
+
+    /**
+     * @return The render-node descriptor, or -1.
+     */
+    int fd() const {
+      return drm_fd;
+    }
+
+    /**
+     * @return `true` when a GBM device is held.
+     */
+    explicit operator bool() const {
+      return device != nullptr;
+    }
+
+  private:
+    gbm_device_accessors_t accessors {};  ///< Functions used to create and destroy the device.
+    int drm_fd {-1};  ///< Render-node descriptor the device was created from, or -1.
+    gbm_device *device {nullptr};  ///< The GBM device, or nullptr.
+  };
+
   /**
    * @brief Captured Wayland frame metadata and DMA-BUF surface state.
    */
@@ -221,7 +301,7 @@ namespace wl {
       std::uint32_t height;
     } dmabuf_info;
 
-    struct gbm_device *gbm_device {nullptr};
+    gbm_device_t gbm;  ///< GBM device and render-node descriptor used to allocate capture buffers.
     struct gbm_bo *current_bo {nullptr};
     struct wl_buffer *current_wl_buffer {nullptr};
     bool y_invert {false};

tests/unit/platform/linux/test_wayland.cpp

@@ -168,4 +168,143 @@ TEST(WaylandCaptureTest, ClosesExportedPlanesAfterLaterPlaneFails) {
   EXPECT_EQ(fd_error, EBADF);
   EXPECT_EQ(frame.sd.fds[0], -1);
 }
+
+namespace {
+  /**
+   * @brief Count of fake GBM devices destroyed by the accessors below (function-local state, reset per test).
+   */
+  int &fake_destroyed_devices() {
+    static int count = 0;
+    return count;
+  }
+
+  /**
+   * @brief Descriptor most recently returned by open_fake_render_node() (reset per test).
+   */
+  int &last_fake_render_node() {
+    static int fd = -1;
+    return fd;
+  }
+
+  /**
+   * @brief Count of create_device calls made through the accessors below (reset per test).
+   */
+  int &fake_create_device_calls() {
+    static int count = 0;
+    return count;
+  }
+
+  int open_fake_render_node(const char *) {
+    last_fake_render_node() = open("/dev/null", O_RDWR | O_CLOEXEC);
+    return last_fake_render_node();
+  }
+
+  int fail_to_open_render_node(const char *) {
+    return -1;
+  }
+
+  gbm_device *create_fake_device(int fd) {
+    ++fake_create_device_calls();
+    // The address of this object stands in for an opaque GBM device.
+    static int storage = 0;
+    return fd >= 0 ? static_cast<gbm_device *>(static_cast<void *>(&storage)) : nullptr;
+  }
+
+  gbm_device *fail_to_create_device(int) {
+    ++fake_create_device_calls();
+    return nullptr;
+  }
+
+  void destroy_fake_device(gbm_device *) {
+    ++fake_destroyed_devices();
+  }
+
+  bool descriptor_is_open(int fd) {
+    return fcntl(fd, F_GETFD) != -1;
+  }
+}  // namespace
+
+TEST(WaylandGbmDeviceTest, ClosesRenderNodeDescriptorOnReset) {
+  const wl::gbm_device_accessors_t accessors {
+    .open_render_node = open_fake_render_node,
+    .create_device = create_fake_device,
+    .destroy_device = destroy_fake_device,
+  };
+  fake_destroyed_devices() = 0;
+
+  wl::gbm_device_t device;
+  ASSERT_TRUE(device.init("/dev/dri/renderD128", accessors));
+  ASSERT_TRUE(device);
+  const int fd = device.fd();
+  ASSERT_GE(fd, 0);
+  EXPECT_TRUE(descriptor_is_open(fd));
+
+  device.reset();
+  EXPECT_FALSE(device);
+  EXPECT_EQ(device.fd(), -1);
+  EXPECT_EQ(fake_destroyed_devices(), 1);
+  EXPECT_FALSE(descriptor_is_open(fd));
+}
+
+TEST(WaylandGbmDeviceTest, ClosesRenderNodeDescriptorWhenDeviceCreationFails) {
+  const wl::gbm_device_accessors_t accessors {
+    .open_render_node = open_fake_render_node,
+    .create_device = fail_to_create_device,
+    .destroy_device = destroy_fake_device,
+  };
+  fake_destroyed_devices() = 0;
+  fake_create_device_calls() = 0;
+  last_fake_render_node() = -1;
+
+  wl::gbm_device_t device;
+  EXPECT_FALSE(device.init("/dev/dri/renderD128", accessors));
+  EXPECT_FALSE(device);
+  EXPECT_EQ(device.fd(), -1);
+  EXPECT_EQ(fake_create_device_calls(), 1);
+  EXPECT_EQ(fake_destroyed_devices(), 0);
+
+  // Check the descriptor the fake accessor actually opened.
+  const int fd = last_fake_render_node();
+  ASSERT_GE(fd, 0);
+  EXPECT_FALSE(descriptor_is_open(fd));
+}
+
+TEST(WaylandGbmDeviceTest, DoesNotCreateDeviceWhenRenderNodeFailsToOpen) {
+  const wl::gbm_device_accessors_t accessors {
+    .open_render_node = fail_to_open_render_node,
+    .create_device = create_fake_device,
+    .destroy_device = destroy_fake_device,
+  };
+  fake_destroyed_devices() = 0;
+  fake_create_device_calls() = 0;
+
+  wl::gbm_device_t device;
+  EXPECT_FALSE(device.init("/dev/dri/renderD128", accessors));
+  EXPECT_FALSE(device);
+  EXPECT_EQ(device.get(), nullptr);
+  EXPECT_EQ(device.fd(), -1);
+  EXPECT_EQ(fake_create_device_calls(), 0);
+  EXPECT_EQ(fake_destroyed_devices(), 0);
+}
+
+TEST(WaylandGbmDeviceTest, DestructorClosesRenderNodeDescriptor) {
+  const wl::gbm_device_accessors_t accessors {
+    .open_render_node = open_fake_render_node,
+    .create_device = create_fake_device,
+    .destroy_device = destroy_fake_device,
+  };
+  fake_destroyed_devices() = 0;
+  int fd = -1;
+
+  {
+    wl::gbm_device_t device;
+    ASSERT_TRUE(device.init("/dev/dri/renderD128", accessors));
+    fd = device.fd();
+    ASSERT_TRUE(descriptor_is_open(fd));
+  }
+
+  EXPECT_EQ(fake_destroyed_devices(), 1);
+  EXPECT_FALSE(descriptor_is_open(fd));
+}
+
 #endif