From 8ca9dbfcd3fe151e80716b5ebbed7355eb323b6c Mon Sep 17 00:00:00 2001 From: Raphael Derosso Pereira Date: Sun, 20 Sep 2026 22:47:38 -0300 Subject: [PATCH 1/2] fix(linux): close the DRM render-node fd owned by wl::dmabuf_t `dmabuf_t::init_gbm()` opens the render node and hands the descriptor to `gbm_create_device()`; the destructor destroys the GBM device but never closes the descriptor, on the assumption that GBM owns it. It does not: libgbm leaves the fd to the caller (`gbm_device_destroy()` does not close it). Every capture (re)initialisation constructs a new `dmabuf_t` -- one per streaming session start, plus one per output mode change and per encoder validation pass that reaches a real capture -- so the process keeps one open `/dev/dri/renderD*` descriptor per (re)init for its lifetime. Measured on a long-running host (Hyprland, `capture = wlr`, `encoder = nvenc`, NVIDIA 610.57.04, v2026.516.143833; unchanged on master): `/dev/dri` descriptors went 4 -> 8 across three Moonlight connect / disconnect cycles and only a restart of Sunshine returned them. Own the descriptor together with the device in a small RAII type, `wl::gbm_device_t`, whose `reset()` destroys the device and then closes the descriptor (and which closes it when device creation fails); open it with `O_CLOEXEC` so prep/do commands do not inherit it. The open/create/ destroy calls are injectable (`gbm_device_accessors_t`, same shape as `gbm_bo_accessors_t`) so the lifecycle is covered by unit tests that need no GPU: the descriptor is closed on reset, on the failure path and on destruction. Signed-off-by: Raphael Derosso Pereira --- src/platform/linux/wayland.cpp | 75 +++++++++++------ src/platform/linux/wayland.h | 82 +++++++++++++++++- tests/unit/platform/linux/test_wayland.cpp | 96 ++++++++++++++++++++++ 3 files changed, 229 insertions(+), 24 deletions(-) diff --git a/src/platform/linux/wayland.cpp b/src/platform/linux/wayland.cpp index 23ddf64c2cc..8353cef3266 100644 --- a/src/platform/linux/wayland.cpp +++ b/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 static auto classCall(void *data, Params... params) -> decltype(((*reinterpret_cast(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 @@ -377,11 +410,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 @@ -457,12 +486,12 @@ namespace wl { if (modifiers_to_use) { auto it = modifiers_to_use->find(dmabuf_info.format); if (it != modifiers_to_use->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) { diff --git a/src/platform/linux/wayland.h b/src/platform/linux/wayland.h index 6c9f6ed07b7..f4d4df95c23 100644 --- a/src/platform/linux/wayland.h +++ b/src/platform/linux/wayland.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #ifdef SUNSHINE_BUILD_WAYLAND @@ -28,6 +29,7 @@ #ifdef SUNSHINE_BUILD_WAYLAND struct gbm_bo; +struct gbm_device; namespace wl { /** @@ -66,6 +68,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. */ @@ -235,7 +315,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}; diff --git a/tests/unit/platform/linux/test_wayland.cpp b/tests/unit/platform/linux/test_wayland.cpp index 98b221a9dde..9dcfc14e311 100644 --- a/tests/unit/platform/linux/test_wayland.cpp +++ b/tests/unit/platform/linux/test_wayland.cpp @@ -168,4 +168,100 @@ 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; + } + + int open_fake_render_node(const char *) { + return open("/dev/null", O_RDWR | O_CLOEXEC); + } + + gbm_device *create_fake_device(int fd) { + // The address of this object stands in for an opaque GBM device. + static int storage = 0; + return fd >= 0 ? static_cast(static_cast(&storage)) : nullptr; + } + + gbm_device *fail_to_create_device(int) { + 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; + + // Learn which descriptor the next open() will return, so its fate can be checked afterwards. + const int probe = open("/dev/null", O_RDONLY | O_CLOEXEC); + ASSERT_GE(probe, 0); + close(probe); + + wl::gbm_device_t device; + EXPECT_FALSE(device.init("/dev/dri/renderD128", accessors)); + EXPECT_FALSE(device); + EXPECT_EQ(device.fd(), -1); + EXPECT_EQ(fake_destroyed_devices(), 0); + EXPECT_FALSE(descriptor_is_open(probe)); +} + +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 From 60ea5b7c577e2252da5e1702c8e409841189fc5f Mon Sep 17 00:00:00 2001 From: Raphael Derosso Pereira Date: Tue, 29 Sep 2026 16:58:54 -0300 Subject: [PATCH 2/2] test(linux): cover the render-node open failure and record the fake's fd ClosesRenderNodeDescriptorWhenDeviceCreationFails no longer predicts the descriptor with a probe open()/close(): open_fake_render_node() records the descriptor it returned, and the test checks that one, so another thread claiming the number between the calls cannot make it flaky or check the wrong fd. DoesNotCreateDeviceWhenRenderNodeFailsToOpen covers the early return in gbm_device_t::init(): with open_render_node returning -1, create_device is never called (counted by the fake accessors), and the owner stays empty (no device, fd() == -1, nothing destroyed). --- tests/unit/platform/linux/test_wayland.cpp | 55 +++++++++++++++++++--- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/tests/unit/platform/linux/test_wayland.cpp b/tests/unit/platform/linux/test_wayland.cpp index 9dcfc14e311..4f58f4f41bb 100644 --- a/tests/unit/platform/linux/test_wayland.cpp +++ b/tests/unit/platform/linux/test_wayland.cpp @@ -178,17 +178,40 @@ namespace { 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 *) { - return open("/dev/null", O_RDWR | O_CLOEXEC); + 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(static_cast(&storage)) : nullptr; } gbm_device *fail_to_create_device(int) { + ++fake_create_device_calls(); return nullptr; } @@ -230,18 +253,38 @@ TEST(WaylandGbmDeviceTest, ClosesRenderNodeDescriptorWhenDeviceCreationFails) { .destroy_device = destroy_fake_device, }; fake_destroyed_devices() = 0; + fake_create_device_calls() = 0; + last_fake_render_node() = -1; - // Learn which descriptor the next open() will return, so its fate can be checked afterwards. - const int probe = open("/dev/null", O_RDONLY | O_CLOEXEC); - ASSERT_GE(probe, 0); - close(probe); + 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); - EXPECT_FALSE(descriptor_is_open(probe)); } TEST(WaylandGbmDeviceTest, DestructorClosesRenderNodeDescriptor) {