fix: try to fix crash in processWaylandEvents#594
fix: try to fix crash in processWaylandEvents#594wineee wants to merge 1 commit intovioken:masterfrom
Conversation
src/server/kernel/wserver.cpp
Outdated
|
|
||
| QAbstractEventDispatcher *dispatcher = QThread::currentThread()->eventDispatcher(); | ||
| QObject::connect(dispatcher, &QAbstractEventDispatcher::aboutToBlock, q, processWaylandEvents); | ||
| QAbstractEventDispatcher *dispatcher = QAbstractEventDispatcher::instance(); |
There was a problem hiding this comment.
The implementation is actually the same as QThread::currentThread()->eventDispatcher();
kwinwrapland(kwinft)qtwayland |
There was a problem hiding this comment.
Pull request overview
This PR addresses a crash in processWaylandEvents by refactoring the event processing code and adding better error handling. The main changes include splitting the inline lambda into separate methods, adding file descriptor validation, and improving logging.
Key changes:
- Split the inline
processWaylandEventslambda into two dedicated methods:dispatchEvents()for event processing andflush()for client flushing - Added file descriptor validation with fatal error logging when
wl_event_loop_get_fd()returns -1 - Migrated from
QDebugtoQLoggingCategoryfor better structured logging with the category "waylib.server.core"
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/server/kernel/wsocket.cpp | Removed unused forward declaration and simplified variable initialization in addClient() |
| src/server/kernel/wserver.cpp | Refactored event processing into separate methods, added FD validation, migrated to QLoggingCategory, and changed event dispatcher retrieval to use QCoreApplication |
| src/server/kernel/private/wserver_p.h | Added override to destructor and declared new dispatchEvents() and flush() methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| int ret = wl_event_loop_dispatch(loop, 0); | ||
| if (ret) | ||
| qCCritical(qLcWlrServer, "wl_event_loop_dispatch error: %d\n", ret); |
There was a problem hiding this comment.
The error message includes a trailing newline character which is unnecessary. Qt's logging macros automatically add newlines. This is inconsistent with other qCCritical and qCFatal usage in the codebase (see line 109 and other files like wforeigntoplevelv1.cpp line 72, wcursorimage.cpp line 666) which don't include the trailing newline.
| qCCritical(qLcWlrServer, "wl_event_loop_dispatch error: %d\n", ret); | |
| qCCritical(qLcWlrServer, "wl_event_loop_dispatch error: %d", ret); |
| @@ -142,6 +144,18 @@ void WServerPrivate::stop() | |||
| QThread::currentThread()->eventDispatcher()->disconnect(q); | |||
There was a problem hiding this comment.
Inconsistent event dispatcher retrieval in the stop() method. The init() method was changed to use QCoreApplication::eventDispatcher() (line 117), but stop() still uses QThread::currentThread()->eventDispatcher(). These should be consistent to avoid potential issues if the event dispatcher changes or if the server is accessed from different threads.
| QThread::currentThread()->eventDispatcher()->disconnect(q); | |
| QCoreApplication::eventDispatcher()->disconnect(q); |
No description provided.