From 21325f93855e7053f562c722247eb3e1c62581e6 Mon Sep 17 00:00:00 2001 From: Tom Englund Date: Tue, 27 Jan 2026 13:11:54 +0100 Subject: [PATCH] eventLoop: various eventloopmgr fixes (#13091) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * eventloopmgr: use unordered_map for readableWaiters use an unordered_map with the raw ptr as key, avoids any risk of dangling ptrs. * eventloopmgr: read the timerfd fd the manpage for timerfd_create and read states this. timefd_create creates a new timer object, and returns a file descriptor that can be used to read the number of expirations that have occurred. The FD becomes readable when the timer expires. read removes the “readable” state from the FD. so most likely it has somewhat worked because of the scheduleRecalc() function. * eventloopmgr: avoid unneeded std::function copy move the idle functions instead of copying. * eventloopmgr: remove event source before calling fn if fn causes a dispatch/reentry its gonna cause UB inside libwayland itself, remove the event source before calling the fn() avoids that entirerly. even if a new dispatch occurs. * eventloopmgr: check if timer fd is readable check if timerfd is readable before calling read on it, so we dont end up blocking on an accident, log an error if its not readable. * eventloopmgr: revert unordered_map change my mistake, the address wasnt changing on reallocations of the heap object. the only issue i was triggering was the reentry path in fn() --- src/managers/eventLoop/EventLoopManager.cpp | 40 ++++++++++++++------- 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/src/managers/eventLoop/EventLoopManager.cpp b/src/managers/eventLoop/EventLoopManager.cpp index 9933edf6..e38474aa 100644 --- a/src/managers/eventLoop/EventLoopManager.cpp +++ b/src/managers/eventLoop/EventLoopManager.cpp @@ -26,10 +26,7 @@ CEventLoopManager::~CEventLoopManager() { wl_event_source_remove(eventSourceData.eventSource); } - for (auto const& w : m_readableWaiters) { - if (w->source != nullptr) - wl_event_source_remove(w->source); - } + m_readableWaiters.clear(); if (m_wayland.eventSource) wl_event_source_remove(m_wayland.eventSource); @@ -40,14 +37,21 @@ CEventLoopManager::~CEventLoopManager() { } static int timerWrite(int fd, uint32_t mask, void* data) { + if (!CFileDescriptor::isReadable(fd)) + Log::logger->log(Log::ERR, "timerWrite: triggered a non readable event on fd : {}", fd); + else { + uint64_t expirations; + read(fd, &expirations, sizeof(expirations)); + } + g_pEventLoopManager->onTimerFire(); - return 1; + return 0; } static int aquamarineFDWrite(int fd, uint32_t mask, void* data) { auto POLLFD = sc(data); POLLFD->onSignal(); - return 1; + return 0; } static int configWatcherWrite(int fd, uint32_t mask, void* data) { @@ -56,14 +60,21 @@ static int configWatcherWrite(int fd, uint32_t mask, void* data) { } static int handleWaiterFD(int fd, uint32_t mask, void* data) { + auto waiter = sc(data); + + if (!waiter) { + Log::logger->log(Log::ERR, "handleWaiterFD: failed casting waiter"); + return 0; + } + if (mask & (WL_EVENT_HANGUP | WL_EVENT_ERROR)) { Log::logger->log(Log::ERR, "handleWaiterFD: readable waiter error"); - g_pEventLoopManager->onFdReadableFail(sc(data)); + g_pEventLoopManager->onFdReadableFail(waiter); return 0; } if (mask & WL_EVENT_READABLE) - g_pEventLoopManager->onFdReadable(sc(data)); + g_pEventLoopManager->onFdReadable(waiter); return 0; } @@ -75,6 +86,11 @@ void CEventLoopManager::onFdReadable(SReadableWaiter* waiter) { if (it == m_readableWaiters.end()) return; + if (waiter->source) { // remove even_source if fn() somehow causes a reentry + wl_event_source_remove(waiter->source); + waiter->source = nullptr; + } + UP taken = std::move(*it); m_readableWaiters.erase(it); @@ -193,12 +209,12 @@ void CEventLoopManager::doLater(const std::function& fn) { m_wayland.loop, [](void* data) { auto IDLE = sc(data); - auto cpy = IDLE->fns; + auto fns = std::move(IDLE->fns); IDLE->fns.clear(); IDLE->eventSource = nullptr; - for (auto const& c : cpy) { - if (c) - c(); + for (auto& f : fns) { + if (f) + f(); } }, &m_idle);