From f6458fa1e8e92fdf16a1acc961703d229894454c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Tue, 7 Jul 2015 17:35:57 +0200 Subject: [PATCH] Port away from KToolInvocation The problem with KToolInvocation is that it creates a dead lock on Wayland in case kdeinit is not already running. It starts kdeinit and does a QProcess::waitForFinished and our kdeinit needs to interact with the wayland server. So dead lock. As KRun also calls into the dangerous code path it's no option which leaves us with QProcess to start the processes. A nice side-effect is that we no don't need to link KF5::Service any more from kwin_core. Now once Plasma and Notification don't use it any more, it will be gone completely. --- CMakeLists.txt | 3 +-- config-kwin.h.cmake | 1 + main.cpp | 5 +++++ main.h | 3 +++ main_wayland.cpp | 5 ++--- main_wayland.h | 4 ++++ rules.cpp | 15 +++++++++++++-- useractions.cpp | 20 ++++++++++++++------ 8 files changed, 43 insertions(+), 13 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a94bbe6e8e..034bb23a8c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -72,7 +72,6 @@ find_package(KF5 ${KF5_MIN_VERSION} REQUIRED COMPONENTS I18n Init Notifications - Service Package Plasma WidgetsAddons @@ -86,6 +85,7 @@ find_package(KF5 ${KF5_MIN_VERSION} REQUIRED COMPONENTS KCMUtils KIO NewStuff + Service XmlGui ) @@ -502,7 +502,6 @@ set(kwin_KDE_LIBS KF5::GlobalAccelPrivate KF5::I18n KF5::Notifications - KF5::Service KF5::Package KF5::Plasma KF5::WindowSystem diff --git a/config-kwin.h.cmake b/config-kwin.h.cmake index 181202c007..6128c0ccae 100644 --- a/config-kwin.h.cmake +++ b/config-kwin.h.cmake @@ -7,6 +7,7 @@ #define KWIN_VERSION_STRING "${PROJECT_VERSION}" #define XCB_VERSION_STRING "${XCB_VERSION}" #define KWIN_KILLER_BIN "${CMAKE_INSTALL_PREFIX}/${LIBEXEC_INSTALL_DIR}/kwin_killer_helper" +#define KWIN_RULES_DIALOG_BIN "${CMAKE_INSTALL_PREFIX}/${LIBEXEC_INSTALL_DIR}/kwin_rules_dialog" #cmakedefine01 HAVE_WAYLAND #cmakedefine01 HAVE_WAYLAND_EGL #cmakedefine01 HAVE_WAYLAND_CURSOR diff --git a/main.cpp b/main.cpp index 53cfbd91a2..2c33f02e52 100644 --- a/main.cpp +++ b/main.cpp @@ -533,6 +533,11 @@ bool Application::usesLibinput() return s_useLibinput; } +QProcessEnvironment Application::processStartupEnvironment() const +{ + return QProcessEnvironment::systemEnvironment(); +} + } // namespace #include "main.moc" diff --git a/main.h b/main.h index c97fd709aa..2f447cd169 100644 --- a/main.h +++ b/main.h @@ -30,6 +30,7 @@ along with this program. If not, see . // Qt #include #include +#include class QCommandLineParser; @@ -141,6 +142,8 @@ public: return m_connection; } + virtual QProcessEnvironment processStartupEnvironment() const; + static void setupMalloc(); static void setupLocalizedString(); static void setupLoggingCategoryFilters(); diff --git a/main_wayland.cpp b/main_wayland.cpp index 5cd353b2d6..9ca37e3997 100644 --- a/main_wayland.cpp +++ b/main_wayland.cpp @@ -189,15 +189,14 @@ void ApplicationWayland::continueStartupWithX() } } + m_environment.insert(QStringLiteral("DISPLAY"), QString::fromUtf8(qgetenv("DISPLAY"))); // start the applications passed to us as command line arguments if (!m_applicationsToStart.isEmpty()) { - QProcessEnvironment environment = m_environment; - environment.insert(QStringLiteral("DISPLAY"), QString::fromUtf8(qgetenv("DISPLAY"))); for (const QString &application: m_applicationsToStart) { // note: this will kill the started process when we exit // this is going to happen anyway as we are the wayland and X server the app connects to QProcess *p = new QProcess(this); - p->setProcessEnvironment(environment); + p->setProcessEnvironment(m_environment); p->start(application); } } diff --git a/main_wayland.h b/main_wayland.h index a7860b8bae..36fb3b5e5c 100644 --- a/main_wayland.h +++ b/main_wayland.h @@ -50,6 +50,10 @@ public: bool notify(QObject *o, QEvent *e) override; + QProcessEnvironment processStartupEnvironment() const override { + return m_environment; + } + protected: void performStartup() override; diff --git a/rules.cpp b/rules.cpp index ac97358e11..93856132bf 100644 --- a/rules.cpp +++ b/rules.cpp @@ -27,7 +27,6 @@ along with this program. If not, see . #include #include #include -#include #include #ifndef KCMRULES @@ -995,7 +994,19 @@ void RuleBook::edit(AbstractClient* c, bool whole_app) args << QStringLiteral("--wid") << QString::number(c->window()); if (whole_app) args << QStringLiteral("--whole-app"); - KToolInvocation::kdeinitExec(QStringLiteral("kwin_rules_dialog"), args); + QProcess *p = new QProcess(this); + p->setArguments(args); + p->setProcessEnvironment(kwinApp()->processStartupEnvironment()); + p->setProgram(QStringLiteral(KWIN_RULES_DIALOG_BIN)); + connect(p, static_cast(&QProcess::finished), this, &QProcess::deleteLater); + connect(p, static_cast(&QProcess::error), this, + [p] (QProcess::ProcessError e) { + if (e == QProcess::FailedToStart) { + qCDebug(KWIN_CORE) << "Failed to start kwin_rules_dialog"; + } + } + ); + p->start(); } void RuleBook::load() diff --git a/useractions.cpp b/useractions.cpp index 428d854665..562255d85c 100755 --- a/useractions.cpp +++ b/useractions.cpp @@ -47,7 +47,6 @@ along with this program. If not, see . #endif #include -#include #include #include @@ -307,14 +306,23 @@ void UserActionsMenu::init() "Window &Manager Settings...")); action->setIcon(QIcon::fromTheme(QStringLiteral("configure"))); connect(action, &QAction::triggered, this, - []() { + [this]() { // opens the KWin configuration QStringList args; args << QStringLiteral("--icon") << QStringLiteral("preferences-system-windows") << configModules(false); - QString error; - if (KToolInvocation::kdeinitExec(QStringLiteral("kcmshell5"), args, &error) != 0) { - qCDebug(KWIN_CORE) << "Failed to start kcmshell5: " << error; - } + QProcess *p = new QProcess(this); + p->setArguments(args); + p->setProcessEnvironment(kwinApp()->processStartupEnvironment()); + p->setProgram(QStringLiteral("kcmshell5")); + connect(p, static_cast(&QProcess::finished), this, &QProcess::deleteLater); + connect(p, static_cast(&QProcess::error), this, + [p] (QProcess::ProcessError e) { + if (e == QProcess::FailedToStart) { + qCDebug(KWIN_CORE) << "Failed to start kcmshell5"; + } + } + ); + p->start(); } ); }