From d8b05e03aa8a52a50ebbf257d30a270f4b30e6fc Mon Sep 17 00:00:00 2001 From: liu weikai Date: Tue, 10 Nov 2026 09:38:32 +0800 Subject: [PATCH] fix(linux): release display resources when initialization fails --- CHANGELOG.md | 10 ++ apps/linux_cardputer_zero/CMakeLists.txt | 12 +++ .../platform/wayland/wayland_presenter.cpp | 97 +++++++++++-------- .../tests/linux_framebuffer_present_test.cpp | 84 +++++++++++++++- .../tests/linux_wayland_resource_test.cpp | 68 +++++++++++++ .../rpi/src/platform/device/evdev_input.cpp | 2 +- .../device/linux_framebuffer_platform.cpp | 61 +++++++----- 7 files changed, 266 insertions(+), 68 deletions(-) create mode 100644 apps/linux_cardputer_zero/tests/linux_wayland_resource_test.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 799ee41f..80b8c48f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,16 @@ stable firmware release. Hardware support and validation remain target-specific. ## [Unreleased] +### Fixed + +- Linux framebuffer initialization closes its device descriptor and mapping + when screen-info queries, pixel-depth validation, mapping or buffer allocation + fail. Framebuffer and keyboard descriptors also close on exec instead of being + inherited by unrelated helper programs; reopening the keyboard retains one + owned descriptor. +- Wayland initialization releases its display connection, protocol objects and + any allocated buffers when a compositor is incomplete or setup fails. + ## [1.1.1] - 2026-11-10 ### Fixed diff --git a/apps/linux_cardputer_zero/CMakeLists.txt b/apps/linux_cardputer_zero/CMakeLists.txt index f4e6739e..261ff492 100644 --- a/apps/linux_cardputer_zero/CMakeLists.txt +++ b/apps/linux_cardputer_zero/CMakeLists.txt @@ -221,6 +221,18 @@ if(TRAIL_MATE_CARDPUTER_ZERO_BUILD_DEVICE AND CMAKE_SYSTEM_NAME STREQUAL "Linux" target_link_options(trailmate_linux_wayland_text_input_test PRIVATE -fsanitize=address,undefined) endif() add_test(NAME trailmate_linux_wayland_text_input COMMAND trailmate_linux_wayland_text_input_test) + add_executable(trailmate_linux_wayland_resource_test + tests/linux_wayland_resource_test.cpp + src/platform/wayland/wayland_presenter.cpp + "${TRAIL_MATE_LINUX_COMMON_SRC_ROOT}/core/canvas.cpp") + target_include_directories(trailmate_linux_wayland_resource_test PRIVATE + "${CMAKE_CURRENT_LIST_DIR}/src" "${TRAIL_MATE_LINUX_COMMON_SRC_ROOT}") + target_link_libraries(trailmate_linux_wayland_resource_test PRIVATE + trailmate_cardputer_zero_wayland_protocol PkgConfig::WAYLAND_SERVER + PkgConfig::XKBCOMMON Threads::Threads) + target_compile_features(trailmate_linux_wayland_resource_test PRIVATE cxx_std_20) + target_compile_options(trailmate_linux_wayland_resource_test PRIVATE -UNDEBUG) + add_test(NAME trailmate_linux_wayland_resources COMMAND trailmate_linux_wayland_resource_test) endif() add_executable(trailmate_cardputer_zero_wayland diff --git a/apps/linux_cardputer_zero/src/platform/wayland/wayland_presenter.cpp b/apps/linux_cardputer_zero/src/platform/wayland/wayland_presenter.cpp index 2c0ba699..ab66c4df 100644 --- a/apps/linux_cardputer_zero/src/platform/wayland/wayland_presenter.cpp +++ b/apps/linux_cardputer_zero/src/platform/wayland/wayland_presenter.cpp @@ -180,55 +180,68 @@ struct WaylandPresenter::Impl throw std::runtime_error("wl_display_connect failed"); } - registry = wl_display_get_registry(display); - wl_registry_add_listener(registry, &kRegistryListener, this); - wl_display_roundtrip(display); - wl_display_roundtrip(display); - - if (compositor == nullptr || shm == nullptr || wm == nullptr) + try { - throw std::runtime_error( - "Wayland compositor is missing wl_compositor, wl_shm, or " - "xdg_wm_base"); - } + registry = wl_display_get_registry(display); + wl_registry_add_listener(registry, &kRegistryListener, this); + wl_display_roundtrip(display); + wl_display_roundtrip(display); - surface = wl_compositor_create_surface(compositor); - if (surface == nullptr) + if (compositor == nullptr || shm == nullptr || wm == nullptr) + { + throw std::runtime_error( + "Wayland compositor is missing wl_compositor, wl_shm, or " + "xdg_wm_base"); + } + + surface = wl_compositor_create_surface(compositor); + if (surface == nullptr) + { + throw std::runtime_error("wl_compositor_create_surface failed"); + } + if (text_manager && seat) text_input = std::make_unique(text_manager, seat, surface, pending_inputs); + + xdg_surface_obj = xdg_wm_base_get_xdg_surface(wm, surface); + xdg_surface_add_listener(xdg_surface_obj, &kSurfaceListener, this); + toplevel = xdg_surface_get_toplevel(xdg_surface_obj); + xdg_toplevel_add_listener(toplevel, &kToplevelListener, this); + xdg_toplevel_set_title(toplevel, "TrailMate"); + xdg_toplevel_set_app_id(toplevel, + "io.github.vicliu624.trailmate"); + xdg_toplevel_set_min_size(toplevel, kWidth, kHeight); + xdg_toplevel_set_max_size(toplevel, kWidth, kHeight); + if (decoration_manager != nullptr) + { + decoration = + zxdg_decoration_manager_v1_get_toplevel_decoration( + decoration_manager, + toplevel); + zxdg_toplevel_decoration_v1_set_mode( + decoration, + ZXDG_TOPLEVEL_DECORATION_V1_MODE_CLIENT_SIDE); + } + + if (!createBuffer(buffers[0]) || !createBuffer(buffers[1])) + { + throw std::runtime_error("Wayland shm buffer allocation failed"); + } + + wl_surface_commit(surface); + wl_display_flush(display); + } + catch (...) { - throw std::runtime_error("wl_compositor_create_surface failed"); + releaseResources(); + throw; } - if (text_manager && seat) text_input = std::make_unique(text_manager, seat, surface, pending_inputs); - - xdg_surface_obj = xdg_wm_base_get_xdg_surface(wm, surface); - xdg_surface_add_listener(xdg_surface_obj, &kSurfaceListener, this); - toplevel = xdg_surface_get_toplevel(xdg_surface_obj); - xdg_toplevel_add_listener(toplevel, &kToplevelListener, this); - xdg_toplevel_set_title(toplevel, "TrailMate"); - xdg_toplevel_set_app_id(toplevel, - "io.github.vicliu624.trailmate"); - xdg_toplevel_set_min_size(toplevel, kWidth, kHeight); - xdg_toplevel_set_max_size(toplevel, kWidth, kHeight); - if (decoration_manager != nullptr) - { - decoration = - zxdg_decoration_manager_v1_get_toplevel_decoration( - decoration_manager, - toplevel); - zxdg_toplevel_decoration_v1_set_mode( - decoration, - ZXDG_TOPLEVEL_DECORATION_V1_MODE_CLIENT_SIDE); - } - - if (!createBuffer(buffers[0]) || !createBuffer(buffers[1])) - { - throw std::runtime_error("Wayland shm buffer allocation failed"); - } - - wl_surface_commit(surface); - wl_display_flush(display); } ~Impl() + { + releaseResources(); + } + + void releaseResources() noexcept { text_input.reset(); if (text_manager) zwp_text_input_manager_v3_destroy(text_manager); diff --git a/apps/linux_cardputer_zero/tests/linux_framebuffer_present_test.cpp b/apps/linux_cardputer_zero/tests/linux_framebuffer_present_test.cpp index c48aea5a..f56e43fd 100644 --- a/apps/linux_cardputer_zero/tests/linux_framebuffer_present_test.cpp +++ b/apps/linux_cardputer_zero/tests/linux_framebuffer_present_test.cpp @@ -1,11 +1,14 @@ +#include "platform/device/evdev_input.h" #include "platform/device/linux_framebuffer_platform.h" #include +#include #include #include #include #include #include #include +#include #include #include #include @@ -13,8 +16,42 @@ namespace { constexpr std::size_t kBytes = 320 * 170 * 2; +enum class Failure +{ + None, + FixedInfo, + VariableInfo, + PixelDepth, + Mapping +}; +Failure failure = Failure::None; + +std::size_t descriptorCount() +{ + std::size_t count = 0; + for (const auto& entry : std::filesystem::directory_iterator("/proc/self/fd")) + { + (void)entry; + ++count; + } + return count; } +void checkPrivateDescriptor(const char* path, int fixture) +{ + bool found = false; + for (const auto& entry : std::filesystem::directory_iterator("/proc/self/fd")) + { + const int descriptor = std::stoi(entry.path().filename().string()); + if (descriptor == fixture || std::filesystem::read_symlink(entry.path()) != path) continue; + const auto flags = fcntl(descriptor, F_GETFD); + assert(flags >= 0 && (flags & FD_CLOEXEC) != 0); + found = true; + } + assert(found); +} +} // namespace + extern "C" int __wrap_ioctl(int, unsigned long request, ...) { va_list arguments; @@ -23,19 +60,29 @@ extern "C" int __wrap_ioctl(int, unsigned long request, ...) va_end(arguments); if (request == FBIOGET_FSCREENINFO) { + if (failure == Failure::FixedInfo) + { + errno = EIO; + return -1; + } auto& info = *static_cast(output); info = {}; - info.line_length = 320 * 2; + info.line_length = failure == Failure::Mapping ? 0 : 320 * 2; info.smem_len = kBytes; return 0; } if (request == FBIOGET_VSCREENINFO) { + if (failure == Failure::VariableInfo) + { + errno = EIO; + return -1; + } auto& info = *static_cast(output); info = {}; info.xres = info.xres_virtual = 320; info.yres = info.yres_virtual = 170; - info.bits_per_pixel = 16; + info.bits_per_pixel = failure == Failure::PixelDepth ? 8 : 16; info.red = {11, 5, 0}; info.green = {5, 6, 0}; info.blue = {0, 5, 0}; @@ -53,8 +100,29 @@ int main() const int fd = mkstemp(path); assert(fd >= 0 && ftruncate(fd, kBytes) == 0); setenv("TRAIL_MATE_INPUT_DEVICE", "/dev/trailmate-test-missing-keyboard", 1); + const auto baseline = descriptorCount(); + for (const auto mode : {Failure::FixedInfo, Failure::VariableInfo, Failure::PixelDepth, Failure::Mapping}) + { + failure = mode; + for (unsigned attempt = 0; attempt < 20; ++attempt) + { + bool failed = false; + try + { + LinuxFramebufferPlatform presenter(path); + } + catch (const std::runtime_error&) + { + failed = true; + } + assert(failed); + assert(descriptorCount() == baseline); + } + } + failure = Failure::None; { LinuxFramebufferPlatform presenter(path); + checkPrivateDescriptor(path, fd); Canvas canvas(320, 170); canvas.clear(rgba(255, 255, 255)); presenter.present(canvas); @@ -107,6 +175,18 @@ int main() assert(presenter.statistics().commits == 101); assert(presenter.statistics().bytes_written == kBytes * 101); } + assert(descriptorCount() == baseline); + { + trailmate::cardputer_zero::platform::device::EvdevInput input(path); + assert(input.isOpen()); + for (unsigned attempt = 0; attempt < 100; ++attempt) + { + checkPrivateDescriptor(path, fd); + assert(descriptorCount() == baseline + 1); + assert(input.reopen(path)); + } + } + assert(descriptorCount() == baseline); close(fd); std::filesystem::remove(path); } diff --git a/apps/linux_cardputer_zero/tests/linux_wayland_resource_test.cpp b/apps/linux_cardputer_zero/tests/linux_wayland_resource_test.cpp new file mode 100644 index 00000000..46b38807 --- /dev/null +++ b/apps/linux_cardputer_zero/tests/linux_wayland_resource_test.cpp @@ -0,0 +1,68 @@ +#include "platform/wayland/wayland_presenter.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace +{ +std::size_t descriptorCount() +{ + std::size_t count = 0; + for (const auto& entry : std::filesystem::directory_iterator("/proc/self/fd")) + { + (void)entry; + ++count; + } + return count; +} + +void rejectIncompleteCompositor() +{ + auto* server = wl_display_create(); + assert(server != nullptr); + int sockets[2]; + assert(socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sockets) == 0); + assert(wl_client_create(server, sockets[0]) != nullptr); + const auto socket = std::to_string(sockets[1]); + assert(setenv("WAYLAND_SOCKET", socket.c_str(), 1) == 0); + std::atomic_bool stopping{false}; + std::thread worker([&] + { + auto* loop = wl_display_get_event_loop(server); + while (!stopping) + { + assert(wl_event_loop_dispatch(loop, 10) >= 0); + wl_display_flush_clients(server); + } }); + bool rejected = false; + try + { + trailmate::cardputer_zero::platform::wayland::WaylandPresenter presenter; + } + catch (const std::runtime_error& error) + { + rejected = std::string(error.what()).find("missing wl_compositor") != std::string::npos; + } + stopping = true; + worker.join(); + wl_display_destroy_clients(server); + wl_display_destroy(server); + assert(rejected); +} +} // namespace + +int main() +{ + const auto baseline = descriptorCount(); + for (unsigned attempt = 0; attempt < 20; ++attempt) + { + rejectIncompleteCompositor(); + assert(descriptorCount() == baseline); + } +} diff --git a/platform/linux/rpi/src/platform/device/evdev_input.cpp b/platform/linux/rpi/src/platform/device/evdev_input.cpp index febef006..1fe0126e 100644 --- a/platform/linux/rpi/src/platform/device/evdev_input.cpp +++ b/platform/linux/rpi/src/platform/device/evdev_input.cpp @@ -446,7 +446,7 @@ struct EvdevInput::Impl return false; } - fd = ::open(path.c_str(), O_RDONLY | O_NONBLOCK); + fd = ::open(path.c_str(), O_RDONLY | O_NONBLOCK | O_CLOEXEC); if (fd < 0) { std::fprintf(stderr, "[evdev] Failed to open %s: %s\n", diff --git a/platform/linux/rpi/src/platform/device/linux_framebuffer_platform.cpp b/platform/linux/rpi/src/platform/device/linux_framebuffer_platform.cpp index 37c5573f..e381f887 100644 --- a/platform/linux/rpi/src/platform/device/linux_framebuffer_platform.cpp +++ b/platform/linux/rpi/src/platform/device/linux_framebuffer_platform.cpp @@ -71,49 +71,64 @@ struct LinuxFramebufferPlatform::Impl "Set TRAIL_MATE_INPUT_DEVICE to override.\n"); } - file_descriptor = ::open(framebuffer_path.c_str(), O_RDWR); + file_descriptor = ::open(framebuffer_path.c_str(), O_RDWR | O_CLOEXEC); if (file_descriptor < 0) { throw makeSystemError("open framebuffer"); } - if (::ioctl(file_descriptor, FBIOGET_FSCREENINFO, &finfo) == -1) + try { - throw makeSystemError("FBIOGET_FSCREENINFO"); - } - if (::ioctl(file_descriptor, FBIOGET_VSCREENINFO, &vinfo) == -1) - { - throw makeSystemError("FBIOGET_VSCREENINFO"); - } + if (::ioctl(file_descriptor, FBIOGET_FSCREENINFO, &finfo) == -1) + { + throw makeSystemError("FBIOGET_FSCREENINFO"); + } + if (::ioctl(file_descriptor, FBIOGET_VSCREENINFO, &vinfo) == -1) + { + throw makeSystemError("FBIOGET_VSCREENINFO"); + } - bytes_per_pixel = static_cast(vinfo.bits_per_pixel / 8U); - if (bytes_per_pixel < 2 || bytes_per_pixel > 4) - { - throw std::runtime_error("unsupported framebuffer pixel depth"); - } + bytes_per_pixel = static_cast(vinfo.bits_per_pixel / 8U); + if (bytes_per_pixel < 2 || bytes_per_pixel > 4) + { + throw std::runtime_error("unsupported framebuffer pixel depth"); + } - const auto mapped_size = static_cast(finfo.line_length) * static_cast(vinfo.yres_virtual); - framebuffer_size = mapped_size; - void* mapped = ::mmap(nullptr, framebuffer_size, PROT_READ | PROT_WRITE, MAP_SHARED, file_descriptor, 0); - if (mapped == MAP_FAILED) - { - framebuffer_memory = nullptr; - throw makeSystemError("mmap framebuffer"); - } + const auto mapped_size = static_cast(finfo.line_length) * static_cast(vinfo.yres_virtual); + framebuffer_size = mapped_size; + void* mapped = ::mmap(nullptr, framebuffer_size, PROT_READ | PROT_WRITE, MAP_SHARED, file_descriptor, 0); + if (mapped == MAP_FAILED) + { + framebuffer_memory = nullptr; + throw makeSystemError("mmap framebuffer"); + } - framebuffer_memory = static_cast(mapped); - staging.resize(framebuffer_size); + framebuffer_memory = static_cast(mapped); + staging.resize(framebuffer_size); + } + catch (...) + { + closeFramebuffer(); + throw; + } } ~Impl() + { + closeFramebuffer(); + } + + void closeFramebuffer() noexcept { if (framebuffer_memory) { ::munmap(framebuffer_memory, framebuffer_size); + framebuffer_memory = nullptr; } if (file_descriptor >= 0) { ::close(file_descriptor); + file_descriptor = -1; } }