From e832479c479d6d991551267be9d2c83a650ed299 Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Wed, 22 Dec 2021 12:26:33 +0100 Subject: [PATCH 01/16] quickfix for shitty written daemon initialization --- daemon/core.cpp | 29 +++++++++++++++++++++++------ daemon/core.h | 6 ++++++ daemon/main.cpp | 6 +----- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/daemon/core.cpp b/daemon/core.cpp index ce2240fc..72bed5a2 100644 --- a/daemon/core.cpp +++ b/daemon/core.cpp @@ -403,6 +403,12 @@ Core::Core(bool useSyslog, bool minLogLevelSet, int minLogLevel, const QStringLi { s_Core = this; +#if 1 // FIXME: Quickfix splitting thread startup. + this->multipleActionsBehaviourSet = multipleActionsBehaviourSet; + this->configFiles = configFiles; + this->minLogLevelSet = minLogLevelSet; +#endif + initBothPipeEnds(mX11ErrorPipe); initBothPipeEnds(mX11RequestPipe); initBothPipeEnds(mX11ResponsePipe); @@ -438,17 +444,24 @@ Core::Core(bool useSyslog, bool minLogLevelSet, int minLogLevel, const QStringLi throw std::runtime_error(std::string("Cannot create X11 response pipe: ") + std::string(strerror(c_error))); } + // FIXME: Quickfix for shitty written async thread initialization. + // IMPORTANT: thread_started will be called from within QCoreApplication::exec() + connect(this, &QThread::started, this, &Core::thread_started, Qt::QueuedConnection); + } catch(const std::exception& err) { + log(LOG_CRIT, "%s", err.what()); + } +} - start(); - - +void Core::thread_started() { + // FIXME: Quickfix for shitty written async thread initialization + try { char signal; - error_t error = readAll(mX11ResponsePipe[STDIN_FILENO], &signal, sizeof(signal)); - if (error > 0) + error_t c_error = readAll(mX11ResponsePipe[STDIN_FILENO], &signal, sizeof(signal)); + if (c_error > 0) { throw std::runtime_error(std::string("Cannot read X11 start signal: ") + std::string(strerror(c_error))); } - if (error < 0) + if (c_error < 0) { throw std::runtime_error(std::string("Cannot read X11 start signal")); } @@ -770,6 +783,10 @@ Core::Core(bool useSyslog, bool minLogLevelSet, int minLogLevel, const QStringLi { log(LOG_CRIT, "%s", err.what()); } + + if (!mReady) { + qApp->exit(EXIT_FAILURE); + } } Core::~Core() diff --git a/daemon/core.h b/daemon/core.h index 9bf9da7a..30b7ed62 100644 --- a/daemon/core.h +++ b/daemon/core.h @@ -87,6 +87,7 @@ class Core : public QThread, public LogTarget Core(const Core&&) = delete; Core& operator =(const Core&) = delete; Core& operator =(const Core&&) = delete; + Q_INVOKABLE void thread_started(); private: using X11Shortcut = QPair; @@ -150,6 +151,11 @@ class Core : public QThread, public LogTarget void shortcutGrabTimedout(); private: +#if 1 // FIXME: Workaround splitting thread startup. + bool minLogLevelSet; + QStringList configFiles; + bool multipleActionsBehaviourSet; +#endif bool enableActionNonGuarded(qulonglong id, bool enabled); QPair addOrRegisterClientAction(const QString &shortcut, const QDBusObjectPath &path, const QString &description, const QString &sender); qulonglong registerClientAction(const QString &shortcut, const QDBusObjectPath &path, const QString &description); diff --git a/daemon/main.cpp b/daemon/main.cpp index c8b74456..1cb58c6d 100644 --- a/daemon/main.cpp +++ b/daemon/main.cpp @@ -236,11 +236,7 @@ int main(int argc, char *argv[]) LXQt::Application app(argc, argv); Core core(runAsDaemon || useSyslog, minLogLevelSet, minLogLevel, configFiles, multipleActionsBehaviourSet, multipleActionsBehaviour); - - if (!core.ready()) - { - return EXIT_FAILURE; - } + core.start(); return app.exec(); } From ddcf7a5369e778ca5ddc8ea7de98540e32b40234 Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Sun, 26 Dec 2021 17:49:47 +0100 Subject: [PATCH 02/16] QThread::started does not work -> shitty implementation relies on QThread::run --- daemon/core.cpp | 9 +++------ daemon/core.h | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/daemon/core.cpp b/daemon/core.cpp index 72bed5a2..32bef811 100644 --- a/daemon/core.cpp +++ b/daemon/core.cpp @@ -443,17 +443,14 @@ Core::Core(bool useSyslog, bool minLogLevelSet, int minLogLevel, const QStringLi { throw std::runtime_error(std::string("Cannot create X11 response pipe: ") + std::string(strerror(c_error))); } - - // FIXME: Quickfix for shitty written async thread initialization. - // IMPORTANT: thread_started will be called from within QCoreApplication::exec() - connect(this, &QThread::started, this, &Core::thread_started, Qt::QueuedConnection); } catch(const std::exception& err) { log(LOG_CRIT, "%s", err.what()); } } -void Core::thread_started() { - // FIXME: Quickfix for shitty written async thread initialization +void Core::start() { + QThread::start(InheritPriority); // FIXME: Quickfix for shitty written thread initialization + try { char signal; error_t c_error = readAll(mX11ResponsePipe[STDIN_FILENO], &signal, sizeof(signal)); diff --git a/daemon/core.h b/daemon/core.h index 30b7ed62..e3cba824 100644 --- a/daemon/core.h +++ b/daemon/core.h @@ -75,6 +75,7 @@ class Core : public QThread, public LogTarget Core(bool useSyslog, bool minLogLevelSet, int minLogLevel, const QStringList &configFiles, bool multipleActionsBehaviourSet, MultipleActionsBehaviour multipleActionsBehaviour, QObject *parent = nullptr); ~Core() override; + void start(); bool ready() const { return mReady; } void log(int level, const char *format, ...) const override; @@ -87,7 +88,6 @@ class Core : public QThread, public LogTarget Core(const Core&&) = delete; Core& operator =(const Core&) = delete; Core& operator =(const Core&&) = delete; - Q_INVOKABLE void thread_started(); private: using X11Shortcut = QPair; From 0db9bf6bc1d284e8274bbe763e8c1914095882a5 Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Sun, 26 Dec 2021 19:16:24 +0100 Subject: [PATCH 03/16] debug issue #247 --- daemon/core.cpp | 31 ++++++++++++++++++++----------- daemon/core.h | 4 +++- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/daemon/core.cpp b/daemon/core.cpp index 32bef811..650792b6 100644 --- a/daemon/core.cpp +++ b/daemon/core.cpp @@ -86,15 +86,6 @@ static const QLatin1String lastStr("last"); static const QLatin1String allStr("all"); static const QLatin1String noneStr("none"); -int x11ErrorHandler(Display *display, XErrorEvent *errorEvent) -{ - if (s_Core) - { - return s_Core->x11ErrorHandler(display, errorEvent); - } - return 0; -} - const char *x11opcodeToString(unsigned char opcode) { switch (opcode) @@ -925,6 +916,18 @@ int Core::x11ErrorHandler(Display */*display*/, XErrorEvent *errorEvent) return 0; } +int Core::x11IoErrorHandler(Display* display) +{ +#if 1 // TODO: Workaround for https://github.com/lxqt/lxqt-globalkeys/issues/247 + if (display == mDisplay) { + log(LOG_INFO, "X IO error ignored -> workaround for https://github.com/lxqt/lxqt-globalkeys/issues/247"); + return 0; + } +#endif + + return mOldXIOErrorHandler(display); +} + bool Core::waitForX11Error(int level, uint timeout) { pollfd fds[1]; @@ -999,7 +1002,12 @@ void Core::run() mX11EventLoopActive = true; XInitThreads(); - int (*oldx11ErrorHandler)(Display * display, XErrorEvent * errorEvent) = XSetErrorHandler(::x11ErrorHandler); + mOldXErrorHandler = XSetErrorHandler([](auto display, auto errorEvent){ + return s_Core ? s_Core->x11ErrorHandler(display, errorEvent) : 0; + }); + mOldXIOErrorHandler = XSetIOErrorHandler([](auto display){ + return s_Core ? s_Core->x11IoErrorHandler(display) : 0; + }); mDisplay = XOpenDisplay(nullptr); XSynchronize(mDisplay, True); @@ -1018,7 +1026,8 @@ void Core::run() lockX11Error(); XUngrabKey(mDisplay, AnyKey, AnyModifier, rootWindow); - XSetErrorHandler(oldx11ErrorHandler); + XSetErrorHandler(mOldXErrorHandler); + XSetIOErrorHandler(mOldXIOErrorHandler); XCloseDisplay(mDisplay); checkX11Error(0); } diff --git a/daemon/core.h b/daemon/core.h index e3cba824..b4b559e2 100644 --- a/daemon/core.h +++ b/daemon/core.h @@ -156,6 +156,8 @@ class Core : public QThread, public LogTarget QStringList configFiles; bool multipleActionsBehaviourSet; #endif + XErrorHandler mOldXErrorHandler = nullptr; + XIOErrorHandler mOldXIOErrorHandler = nullptr; bool enableActionNonGuarded(qulonglong id, bool enabled); QPair addOrRegisterClientAction(const QString &shortcut, const QDBusObjectPath &path, const QString &description, const QString &sender); qulonglong registerClientAction(const QString &shortcut, const QDBusObjectPath &path, const QString &description); @@ -167,8 +169,8 @@ class Core : public QThread, public LogTarget friend void unixSignalHandler(int signalNumber); void unixSignalHandler(int signalNumber); - friend int x11ErrorHandler(Display *display, XErrorEvent *errorEvent); int x11ErrorHandler(Display *display, XErrorEvent *errorEvent); + int x11IoErrorHandler(Display* display); X11Shortcut ShortcutToX11(const QString &shortcut); QString X11ToShortcut(const X11Shortcut &X11shortcut); From d99caa8852c8e497cdc73f1ff527b9fa2c78a875 Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Mon, 27 Dec 2021 21:10:22 +0100 Subject: [PATCH 04/16] prevent calls to wakeX11Thread from within the event listener thread --- daemon/core.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/daemon/core.cpp b/daemon/core.cpp index 650792b6..6b194fbb 100644 --- a/daemon/core.cpp +++ b/daemon/core.cpp @@ -982,6 +982,10 @@ bool Core::checkX11Error(int level, uint timeout) void Core::wakeX11Thread() { + if (currentThread() != qApp->thread()) { + log(LOG_DEBUG, "Core::wakeX11Thread can only be called from main thread"); + return; + } if (mInterClientCommunicationWindow) { XClientMessageEvent dummyEvent; From fdb6f209d1049e6c1ce5326d2c33c76f686a4824 Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Mon, 27 Dec 2021 22:51:03 +0100 Subject: [PATCH 05/16] Just output a more verbose error message and use default error handler. --- daemon/core.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/daemon/core.cpp b/daemon/core.cpp index 6b194fbb..bc4e940c 100644 --- a/daemon/core.cpp +++ b/daemon/core.cpp @@ -918,13 +918,7 @@ int Core::x11ErrorHandler(Display */*display*/, XErrorEvent *errorEvent) int Core::x11IoErrorHandler(Display* display) { -#if 1 // TODO: Workaround for https://github.com/lxqt/lxqt-globalkeys/issues/247 - if (display == mDisplay) { - log(LOG_INFO, "X IO error ignored -> workaround for https://github.com/lxqt/lxqt-globalkeys/issues/247"); - return 0; - } -#endif - + log(LOG_CRIT, "X IO error -> May be issue https://github.com/lxqt/lxqt-globalkeys/issues/247"); return mOldXIOErrorHandler(display); } From 4a76291dd09d5e81a10f96b59df934f79c3a1fe0 Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Mon, 27 Dec 2021 23:06:27 +0100 Subject: [PATCH 06/16] improve readability of current thread check --- daemon/core.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/daemon/core.cpp b/daemon/core.cpp index bc4e940c..b7c9bfb4 100644 --- a/daemon/core.cpp +++ b/daemon/core.cpp @@ -976,8 +976,8 @@ bool Core::checkX11Error(int level, uint timeout) void Core::wakeX11Thread() { - if (currentThread() != qApp->thread()) { - log(LOG_DEBUG, "Core::wakeX11Thread can only be called from main thread"); + if (currentThread() == this) { + log(LOG_DEBUG, "XEvent listener is not waiting. Not waking X11 thread."); return; } if (mInterClientCommunicationWindow) From 33ca1fe6f88f8c420a9197dfeaa4b4ded94818ea Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Mon, 27 Dec 2021 23:26:37 +0100 Subject: [PATCH 07/16] be more verbose in logging X calls --- daemon/core.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/daemon/core.cpp b/daemon/core.cpp index b7c9bfb4..a562e1f4 100644 --- a/daemon/core.cpp +++ b/daemon/core.cpp @@ -440,6 +440,7 @@ Core::Core(bool useSyslog, bool minLogLevelSet, int minLogLevel, const QStringLi } void Core::start() { + log(LOG_DEBUG, "Starting XEvent listener [0]"); QThread::start(InheritPriority); // FIXME: Quickfix for shitty written thread initialization try { @@ -989,14 +990,19 @@ void Core::wakeX11Thread() dummyEvent.format = 32; lockX11Error(); + log(LOG_DEBUG, "Core::wakeX11Thread XSendEvent [0]"); XSendEvent(mDisplay, mInterClientCommunicationWindow, 0, 0, reinterpret_cast(&dummyEvent)); + log(LOG_DEBUG, "Core::wakeX11Thread XSendEvent [1]"); checkX11Error(); + log(LOG_DEBUG, "Core::wakeX11Thread XFlush [0]"); XFlush(mDisplay); + log(LOG_DEBUG, "Core::wakeX11Thread XFlush [1]"); } } void Core::run() { + log(LOG_DEBUG, "Starting XEvent listener [1]"); mX11EventLoopActive = true; XInitThreads(); @@ -1020,7 +1026,9 @@ void Core::run() return; } + log(LOG_DEBUG, "Starting XEvent listener [2]"); runEventLoop(rootWindow); + log(LOG_DEBUG, "XEvent listener finished"); lockX11Error(); XUngrabKey(mDisplay, AnyKey, AnyModifier, rootWindow); From d356822aeaa2adf0e4d176138cc6966255586efb Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Tue, 28 Dec 2021 15:16:22 +0100 Subject: [PATCH 08/16] add logging around XPeekEvent --- daemon/core.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/daemon/core.cpp b/daemon/core.cpp index a562e1f4..b8231d24 100644 --- a/daemon/core.cpp +++ b/daemon/core.cpp @@ -970,7 +970,9 @@ bool Core::checkX11Error(int level, uint timeout) // unsigned long serial = NextRequest(mDisplay); // log(LOG_DEBUG, "X11 error: serial: %lu", serial); + log(LOG_DEBUG, "Core::checkX11Error -> waitForX11Error [0]"); bool result = waitForX11Error(level, timeout); + log(LOG_DEBUG, "Core::checkX11Error -> waitForX11Error [1]"); mX11ErrorMutex.unlock(); return result; } @@ -1058,7 +1060,9 @@ void Core::runEventLoop(Window rootWindow) while (mX11EventLoopActive) { + log(LOG_DEBUG, "Core::run -> XPeekEvent [waiting…]"); XPeekEvent(mDisplay, &event); + log(LOG_DEBUG, "Core::run -> XPeekEvent [event]"); if (!mX11EventLoopActive) { break; From 9b49b7a89d9e97bcfae1b4a63151e8f67441988d Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Tue, 28 Dec 2021 15:16:57 +0100 Subject: [PATCH 09/16] disable XFlush for testing --- daemon/core.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/daemon/core.cpp b/daemon/core.cpp index b8231d24..8cdaef93 100644 --- a/daemon/core.cpp +++ b/daemon/core.cpp @@ -996,9 +996,11 @@ void Core::wakeX11Thread() XSendEvent(mDisplay, mInterClientCommunicationWindow, 0, 0, reinterpret_cast(&dummyEvent)); log(LOG_DEBUG, "Core::wakeX11Thread XSendEvent [1]"); checkX11Error(); +#if 0 // Why do we need XFlush here? log(LOG_DEBUG, "Core::wakeX11Thread XFlush [0]"); XFlush(mDisplay); log(LOG_DEBUG, "Core::wakeX11Thread XFlush [1]"); +#endif } } @@ -1017,8 +1019,8 @@ void Core::run() mDisplay = XOpenDisplay(nullptr); XSynchronize(mDisplay, True); - lockX11Error(); + lockX11Error(); Window rootWindow = DefaultRootWindow(mDisplay); XSelectInput(mDisplay, rootWindow, KeyPressMask | KeyReleaseMask); mInterClientCommunicationWindow = XCreateSimpleWindow(mDisplay, rootWindow, 0, 0, 1, 1, 0, 0, 0); From 646aeca3acf43a3355f5a23afa51b784b227710f Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Tue, 28 Dec 2021 17:27:12 +0100 Subject: [PATCH 10/16] correct (de-)initialization of XEvent listener - XInitThreads (and X object creation) is correctly done in main thread. - No longer segfaults when another daemon is already running. --- daemon/core.cpp | 43 +++++++++++++++++++++++-------------------- daemon/core.h | 3 +++ daemon/main.cpp | 3 +++ 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/daemon/core.cpp b/daemon/core.cpp index 8cdaef93..8ea41f8b 100644 --- a/daemon/core.cpp +++ b/daemon/core.cpp @@ -441,6 +441,10 @@ Core::Core(bool useSyslog, bool minLogLevelSet, int minLogLevel, const QStringLi void Core::start() { log(LOG_DEBUG, "Starting XEvent listener [0]"); + if (initXEventListener()) { + log(LOG_CRIT, "Failed to initialize XEevent listener."); + return; + } QThread::start(InheritPriority); // FIXME: Quickfix for shitty written thread initialization try { @@ -772,10 +776,6 @@ void Core::start() { { log(LOG_CRIT, "%s", err.what()); } - - if (!mReady) { - qApp->exit(EXIT_FAILURE); - } } Core::~Core() @@ -789,6 +789,7 @@ Core::~Core() mX11EventLoopActive = false; wakeX11Thread(); wait(); + deinitXEventListener(); delete mDaemonAdaptor; @@ -1004,10 +1005,18 @@ void Core::wakeX11Thread() } } -void Core::run() +void Core::deinitXEventListener() +{ + lockX11Error(); + XUngrabKey(mDisplay, AnyKey, AnyModifier, rootWindow); + XSetErrorHandler(mOldXErrorHandler); + XSetIOErrorHandler(mOldXIOErrorHandler); + XCloseDisplay(mDisplay); + checkX11Error(LOG_EMERG); +} + +int Core::initXEventListener() { - log(LOG_DEBUG, "Starting XEvent listener [1]"); - mX11EventLoopActive = true; XInitThreads(); mOldXErrorHandler = XSetErrorHandler([](auto display, auto errorEvent){ @@ -1021,25 +1030,19 @@ void Core::run() XSynchronize(mDisplay, True); lockX11Error(); - Window rootWindow = DefaultRootWindow(mDisplay); + this->rootWindow = DefaultRootWindow(mDisplay); XSelectInput(mDisplay, rootWindow, KeyPressMask | KeyReleaseMask); mInterClientCommunicationWindow = XCreateSimpleWindow(mDisplay, rootWindow, 0, 0, 1, 1, 0, 0, 0); XSelectInput(mDisplay, mInterClientCommunicationWindow, StructureNotifyMask); - if (checkX11Error()) - { - return; - } + return checkX11Error(); +} - log(LOG_DEBUG, "Starting XEvent listener [2]"); +void Core::run() +{ + log(LOG_DEBUG, "Starting XEvent listener [1]"); + mX11EventLoopActive = true; runEventLoop(rootWindow); log(LOG_DEBUG, "XEvent listener finished"); - - lockX11Error(); - XUngrabKey(mDisplay, AnyKey, AnyModifier, rootWindow); - XSetErrorHandler(mOldXErrorHandler); - XSetIOErrorHandler(mOldXIOErrorHandler); - XCloseDisplay(mDisplay); - checkX11Error(0); } void Core::runEventLoop(Window rootWindow) diff --git a/daemon/core.h b/daemon/core.h index b4b559e2..2b96f0f4 100644 --- a/daemon/core.h +++ b/daemon/core.h @@ -155,6 +155,7 @@ class Core : public QThread, public LogTarget bool minLogLevelSet; QStringList configFiles; bool multipleActionsBehaviourSet; + Window rootWindow = 0; #endif XErrorHandler mOldXErrorHandler = nullptr; XIOErrorHandler mOldXIOErrorHandler = nullptr; @@ -177,6 +178,8 @@ class Core : public QThread, public LogTarget void wakeX11Thread(); + void deinitXEventListener(); + int initXEventListener(); void run() override; void runEventLoop(Window rootWindow); diff --git a/daemon/main.cpp b/daemon/main.cpp index 1cb58c6d..67c3b9ec 100644 --- a/daemon/main.cpp +++ b/daemon/main.cpp @@ -237,6 +237,9 @@ int main(int argc, char *argv[]) Core core(runAsDaemon || useSyslog, minLogLevelSet, minLogLevel, configFiles, multipleActionsBehaviourSet, multipleActionsBehaviour); core.start(); + if (!core.ready()) { + return EXIT_FAILURE; + } return app.exec(); } From b91ad426140e8a12059621ea1f9565e0fd32dc9a Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Wed, 29 Dec 2021 20:48:21 +0100 Subject: [PATCH 11/16] use qt mutex --- daemon/core.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/daemon/core.cpp b/daemon/core.cpp index 8ea41f8b..bab501bd 100644 --- a/daemon/core.cpp +++ b/daemon/core.cpp @@ -44,7 +44,6 @@ #include #include #include -#include #include @@ -1083,7 +1082,7 @@ void Core::runEventLoop(Window rootWindow) if (((event.type == KeyPress) || (event.type == KeyRelease)) && mDataMutex.tryLock(0)) { - std::unique_lock unlocker(mDataMutex, std::adopt_lock); + mDataMutex.unlock(); // pop event from the x11 queue and process it XNextEvent(mDisplay, &event); From 1ecc8c66cc2593fd3b7bad92dfd522918a45d305 Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Thu, 30 Dec 2021 14:57:03 +0100 Subject: [PATCH 12/16] use a mutex locker on mX11ErrorMutex --- daemon/core.cpp | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/daemon/core.cpp b/daemon/core.cpp index bab501bd..82229eb4 100644 --- a/daemon/core.cpp +++ b/daemon/core.cpp @@ -925,6 +925,7 @@ int Core::x11IoErrorHandler(Display* display) bool Core::waitForX11Error(int level, uint timeout) { + QMutexLocker locker(&mX11ErrorMutex); pollfd fds[1]; fds[0].fd = mX11ErrorPipe[STDIN_FILENO]; fds[0].events = POLLIN | POLLERR | POLLHUP; @@ -961,20 +962,12 @@ bool Core::waitForX11Error(int level, uint timeout) void Core::lockX11Error() { - mX11ErrorMutex.lock(); waitForX11Error(false, 0); } bool Core::checkX11Error(int level, uint timeout) { -// unsigned long serial = NextRequest(mDisplay); -// log(LOG_DEBUG, "X11 error: serial: %lu", serial); - - log(LOG_DEBUG, "Core::checkX11Error -> waitForX11Error [0]"); - bool result = waitForX11Error(level, timeout); - log(LOG_DEBUG, "Core::checkX11Error -> waitForX11Error [1]"); - mX11ErrorMutex.unlock(); - return result; + return waitForX11Error(level, timeout); } void Core::wakeX11Thread() From f60f4eb3804ace310117ca298db7a842b83e1c9c Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Thu, 30 Dec 2021 17:28:18 +0100 Subject: [PATCH 13/16] set object name on QThread --- daemon/main.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/daemon/main.cpp b/daemon/main.cpp index 67c3b9ec..ad174260 100644 --- a/daemon/main.cpp +++ b/daemon/main.cpp @@ -236,6 +236,7 @@ int main(int argc, char *argv[]) LXQt::Application app(argc, argv); Core core(runAsDaemon || useSyslog, minLogLevelSet, minLogLevel, configFiles, multipleActionsBehaviourSet, multipleActionsBehaviour); + core.setObjectName(QStringLiteral("XEvent listener")); core.start(); if (!core.ready()) { return EXIT_FAILURE; From a19fd41b8db49be4b05504c77d452ed63277cd5b Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Thu, 30 Dec 2021 18:42:02 +0100 Subject: [PATCH 14/16] invert mDataMutex locking logic (lock from the main thread only!) Seems to work. Please test it well so we don't introduce anything harmful! --- daemon/core.cpp | 64 +++++++++++++++++++++++++++++++++++-------------- daemon/core.h | 6 +++++ 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/daemon/core.cpp b/daemon/core.cpp index 82229eb4..6eae380c 100644 --- a/daemon/core.cpp +++ b/daemon/core.cpp @@ -1059,6 +1059,7 @@ void Core::runEventLoop(Window rootWindow) { log(LOG_DEBUG, "Core::run -> XPeekEvent [waiting…]"); XPeekEvent(mDisplay, &event); + //QMutexLocker lock(&mDataMutex); log(LOG_DEBUG, "Core::run -> XPeekEvent [event]"); if (!mX11EventLoopActive) { @@ -1072,10 +1073,9 @@ void Core::runEventLoop(Window rootWindow) continue; } keyReleaseExpected = false; // Close time window for accepting meta keys. - - if (((event.type == KeyPress) || (event.type == KeyRelease)) && mDataMutex.tryLock(0)) + if (((event.type == KeyPress) || (event.type == KeyRelease))) { - mDataMutex.unlock(); + //mDataMutex.unlock(); // pop event from the x11 queue and process it XNextEvent(mDisplay, &event); @@ -1589,9 +1589,9 @@ void Core::runEventLoop(Window rootWindow) mX11EventLoopActive = false; break; } - mDataMutex.lock(); + //mDataMutex.lock(); mGrabbingShortcut = true; - mDataMutex.unlock(); + //mDataMutex.unlock(); } break; @@ -1610,9 +1610,9 @@ void Core::runEventLoop(Window rootWindow) break; } - mDataMutex.lock(); + //mDataMutex.lock(); mGrabbingShortcut = false; - mDataMutex.unlock(); + //mDataMutex.unlock(); } break; } // end of switch-case @@ -2036,7 +2036,6 @@ QString Core::checkShortcut(const QString &shortcut, X11Shortcut &X11shortcut) QPair Core::addOrRegisterClientAction(const QString &shortcut, const QDBusObjectPath &path, const QString &description, const QString &sender) { X11Shortcut X11shortcut; - QString newShortcut = checkShortcut(shortcut, X11shortcut); // if (newShortcut.isEmpty()) // { @@ -2083,9 +2082,13 @@ QPair Core::addOrRegisterClientAction(const QString &shortc void Core::addClientAction(QPair &result, const QString &shortcut, const QDBusObjectPath &path, const QString &description, const QString &sender) { - log(LOG_INFO, "addClientAction shortcut:'%s' path:'%s' description:'%s' sender:'%s'", qPrintable(shortcut), qPrintable(path.path()), qPrintable(description), qPrintable(sender)); - QMutexLocker lock(&mDataMutex); + addClientActionInternal(result, shortcut, path, description, sender); +} + +void Core::addClientActionInternal(QPair &result, const QString &shortcut, const QDBusObjectPath &path, const QString &description, const QString &sender) +{ + log(LOG_INFO, "addClientAction shortcut:'%s' path:'%s' description:'%s' sender:'%s'", qPrintable(shortcut), qPrintable(path.path()), qPrintable(description), qPrintable(sender)); SenderByClientPath::iterator senderByClientPath = mSenderByClientPath.find(path); if (senderByClientPath != mSenderByClientPath.end()) @@ -2126,18 +2129,25 @@ void Core::addClientAction(QPair &result, const QString &sh qulonglong Core::registerClientAction(const QString &shortcut, const QDBusObjectPath &path, const QString &description) { - log(LOG_INFO, "registerClientAction shortcut:'%s' path:'%s' description:'%s'", qPrintable(shortcut), qPrintable(path.path()), qPrintable(description)); - QMutexLocker lock(&mDataMutex); + return registerClientActionInternal(shortcut, path, description); +} +qulonglong Core::registerClientActionInternal(const QString &shortcut, const QDBusObjectPath &path, const QString &description) +{ + log(LOG_INFO, "registerClientAction shortcut:'%s' path:'%s' description:'%s'", qPrintable(shortcut), qPrintable(path.path()), qPrintable(description)); return addOrRegisterClientAction(shortcut, path, description, QString()).second; } void Core::addMethodAction(QPair &result, const QString &shortcut, const QString &service, const QDBusObjectPath &path, const QString &interface, const QString &method, const QString &description) { - log(LOG_INFO, "addMethodAction shortcut:'%s' service:'%s' path:'%s' interface:'%s' method:'%s' description:'%s'", qPrintable(shortcut), qPrintable(service), qPrintable(path.path()), qPrintable(interface), qPrintable(method), qPrintable(description)); - QMutexLocker lock(&mDataMutex); + addMethodActionInternal(result, shortcut, service, path, interface, method, description); +} + +void Core::addMethodActionInternal(QPair &result, const QString &shortcut, const QString &service, const QDBusObjectPath &path, const QString &interface, const QString &method, const QString &description) +{ + log(LOG_INFO, "addMethodAction shortcut:'%s' service:'%s' path:'%s' interface:'%s' method:'%s' description:'%s'", qPrintable(shortcut), qPrintable(service), qPrintable(path.path()), qPrintable(interface), qPrintable(method), qPrintable(description)); X11Shortcut X11shortcut; QString newShortcut = checkShortcut(shortcut, X11shortcut); @@ -2169,16 +2179,27 @@ void Core::addMethodAction(QPair &result, const QString &sh qulonglong Core::registerMethodAction(const QString &shortcut, const QString &service, const QDBusObjectPath &path, const QString &interface, const QString &method, const QString &description) { + QMutexLocker lock(&mDataMutex); + return registerMethodActionInternal(shortcut, service, path, interface, method, description); +} + +qulonglong Core::registerMethodActionInternal(const QString &shortcut, const QString &service, const QDBusObjectPath &path, const QString &interface, const QString &method, const QString &description) +{ + log(LOG_INFO, "registerMethodAction shortcut:'%s' path:'%s' description:'%s'", qPrintable(shortcut), qPrintable(path.path()), qPrintable(description)); QPair result; - addMethodAction(result, shortcut, service, path, interface, method, description); + addMethodActionInternal(result, shortcut, service, path, interface, method, description); return result.second; } void Core::addCommandAction(QPair &result, const QString &shortcut, const QString &command, const QStringList &arguments, const QString &description) { - log(LOG_INFO, "addCommandAction shortcut:'%s' command:'%s' arguments:'%s' description:'%s'", qPrintable(shortcut), qPrintable(command), qPrintable(joinToString(arguments, QString(), QLatin1String("' '"), QString())), qPrintable(description)); - QMutexLocker lock(&mDataMutex); + addCommandActionInternal(result, shortcut, command, arguments, description); +} + +void Core::addCommandActionInternal(QPair &result, const QString &shortcut, const QString &command, const QStringList &arguments, const QString &description) +{ + log(LOG_INFO, "addCommandAction shortcut:'%s' command:'%s' arguments:'%s' description:'%s'", qPrintable(shortcut), qPrintable(command), qPrintable(joinToString(arguments, QString(), QLatin1String("' '"), QString())), qPrintable(description)); X11Shortcut X11shortcut; QString newShortcut = checkShortcut(shortcut, X11shortcut); @@ -2210,8 +2231,15 @@ void Core::addCommandAction(QPair &result, const QString &s qulonglong Core::registerCommandAction(const QString &shortcut, const QString &command, const QStringList &arguments, const QString &description) { + QMutexLocker lock(&mDataMutex); + return registerCommandActionInternal(shortcut, command, arguments, description); +} + +qulonglong Core::registerCommandActionInternal(const QString &shortcut, const QString &command, const QStringList &arguments, const QString &description) +{ + log(LOG_INFO, "registerCommandAction shortcut:'%s' arguments:'%s' description:'%s'", qPrintable(shortcut), qPrintable(arguments.join(QLatin1Char(' '))), qPrintable(description)); QPair result; - addCommandAction(result, shortcut, command, arguments, description); + addCommandActionInternal(result, shortcut, command, arguments, description); return result.second; } diff --git a/daemon/core.h b/daemon/core.h index 2b96f0f4..01fd5100 100644 --- a/daemon/core.h +++ b/daemon/core.h @@ -107,8 +107,11 @@ class Core : public QThread, public LogTarget void serviceDisappeared(const QString &sender); void addClientAction(QPair &result, const QString &shortcut, const QDBusObjectPath &path, const QString &description, const QString &sender); + void addClientActionInternal(QPair &result, const QString &shortcut, const QDBusObjectPath &path, const QString &description, const QString &sender); void addMethodAction(QPair &result, const QString &shortcut, const QString &service, const QDBusObjectPath &path, const QString &interface, const QString &method, const QString &description); + void addMethodActionInternal(QPair &result, const QString &shortcut, const QString &service, const QDBusObjectPath &path, const QString &interface, const QString &method, const QString &description); void addCommandAction(QPair &result, const QString &shortcut, const QString &command, const QStringList &arguments, const QString &description); + void addCommandActionInternal(QPair &result, const QString &shortcut, const QString &command, const QStringList &arguments, const QString &description); void modifyClientAction(qulonglong &result, const QDBusObjectPath &path, const QString &description, const QString &sender); void modifyActionDescription(bool &result, const qulonglong &id, const QString &description); @@ -162,8 +165,11 @@ class Core : public QThread, public LogTarget bool enableActionNonGuarded(qulonglong id, bool enabled); QPair addOrRegisterClientAction(const QString &shortcut, const QDBusObjectPath &path, const QString &description, const QString &sender); qulonglong registerClientAction(const QString &shortcut, const QDBusObjectPath &path, const QString &description); + qulonglong registerClientActionInternal(const QString &shortcut, const QDBusObjectPath &path, const QString &description); qulonglong registerMethodAction(const QString &shortcut, const QString &service, const QDBusObjectPath &path, const QString &interface, const QString &method, const QString &description); + qulonglong registerMethodActionInternal(const QString &shortcut, const QString &service, const QDBusObjectPath &path, const QString &interface, const QString &method, const QString &description); qulonglong registerCommandAction(const QString &shortcut, const QString &command, const QStringList &arguments, const QString &description); + qulonglong registerCommandActionInternal(const QString &shortcut, const QString &command, const QStringList &arguments, const QString &description); GeneralActionInfo actionInfo(const ShortcutAndAction &shortcutAndAction) const; From 21a3a038525398b0a664496715e360da4c8765d7 Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Fri, 31 Dec 2021 02:45:41 +0100 Subject: [PATCH 15/16] call XInitThreads before QApplication instance --- daemon/core.cpp | 2 +- daemon/main.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/daemon/core.cpp b/daemon/core.cpp index 6eae380c..2c6e93e2 100644 --- a/daemon/core.cpp +++ b/daemon/core.cpp @@ -1009,7 +1009,7 @@ void Core::deinitXEventListener() int Core::initXEventListener() { - XInitThreads(); + //XInitThreads(); mOldXErrorHandler = XSetErrorHandler([](auto display, auto errorEvent){ return s_Core ? s_Core->x11ErrorHandler(display, errorEvent) : 0; diff --git a/daemon/main.cpp b/daemon/main.cpp index ad174260..70c56978 100644 --- a/daemon/main.cpp +++ b/daemon/main.cpp @@ -233,6 +233,7 @@ int main(int argc, char *argv[]) int ignoreIt = chdir((home && *home) ? home : "/"); (void)ignoreIt; + XInitThreads(); LXQt::Application app(argc, argv); Core core(runAsDaemon || useSyslog, minLogLevelSet, minLogLevel, configFiles, multipleActionsBehaviourSet, multipleActionsBehaviour); From f2da5f2056d1e3fe84eefaa40113e60a639c1be6 Mon Sep 17 00:00:00 2001 From: Nils Fenner Date: Sat, 1 Jan 2022 13:55:47 +0100 Subject: [PATCH 16/16] remove XSynchronize call --- daemon/core.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/daemon/core.cpp b/daemon/core.cpp index 2c6e93e2..ae40e676 100644 --- a/daemon/core.cpp +++ b/daemon/core.cpp @@ -987,6 +987,7 @@ void Core::wakeX11Thread() lockX11Error(); log(LOG_DEBUG, "Core::wakeX11Thread XSendEvent [0]"); XSendEvent(mDisplay, mInterClientCommunicationWindow, 0, 0, reinterpret_cast(&dummyEvent)); + XFlush(mDisplay); log(LOG_DEBUG, "Core::wakeX11Thread XSendEvent [1]"); checkX11Error(); #if 0 // Why do we need XFlush here? @@ -1019,7 +1020,7 @@ int Core::initXEventListener() }); mDisplay = XOpenDisplay(nullptr); - XSynchronize(mDisplay, True); + //XSynchronize(mDisplay, True); lockX11Error(); this->rootWindow = DefaultRootWindow(mDisplay);