From 31777a980e0f4b1b258713158524c0cd9a1dc30f Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Fri, 18 Oct 2019 19:33:20 +0200 Subject: Use execv() to start selected session Prevent vmchooser from lingering in the background, like a zombie, haunting you with invisible windows. Also seems to be notably faster when launching the openbox session. --- src/dialog.cpp | 91 ++++++++++++++++++++------------------------------- src/session.h | 3 +- src/userconfig.cpp | 7 ++++ src/userconfig.h | 1 + src/vsession.cpp | 62 +++++++++++++++++++---------------- src/vsession.h | 4 ++- src/windowmanager.cpp | 14 +++++--- src/windowmanager.h | 2 +- src/xsession.cpp | 35 +++++++++++++++----- src/xsession.h | 9 ++--- 10 files changed, 124 insertions(+), 104 deletions(-) (limited to 'src') diff --git a/src/dialog.cpp b/src/dialog.cpp index 914448f..dbc4b8e 100644 --- a/src/dialog.cpp +++ b/src/dialog.cpp @@ -136,9 +136,11 @@ void Dialog::on_treeView_doubleClicked(const QModelIndex& index) return; // Check preconditions. The method might show error messages and return false - if (!s->canRun(true)) + if (!s->prepareRun()) return; + WindowManager::stopOwnInstance(true); + // These two are up here in case run-virt cares... if (Config::isSet(Config::PVS)) { if (ui->PVS_checkbox->isChecked()) { @@ -159,62 +161,41 @@ void Dialog::on_treeView_doubleClicked(const QModelIndex& index) setenv("VMCHOOSER_DISABLE_SCREENSAVER", "FALSE", 1); } - // Run session - if (s->run()) { - // Run session start script if the session could be initialized successfully - if (QFile::exists(SESSION_START_SCRIPT)) { - // Use the current environment variables and add the necessary - // information for the startUpScipt. - // export session information to the environment of the process to be exec'ed - setenv("SESSION_NAME", s->shortDescription().toUtf8(), 1); - setenv("SESSION_UUID", s->uuid().toUtf8(), 1); - setenv("SESSION_CMD", s->execCommand().toUtf8(), 1); - if (s->type() == Session::VSESSION) { - setenv("SESSION_TYPE", "VSESSION", 1); - } else if (s->type() == Session::XSESSION) { - setenv("SESSION_TYPE", "XSESSION", 1); - } - // Fork this process twice to detach - pid_t pid = fork(); - if (pid == 0) { - - // Close all filedescriptors - for (int i = 3; i < 1024; ++i) - ::close(i); - - // Change process group - setsid(); - - // Reopen standard pipes - freopen("/dev/null", "r", stdin); - freopen("/dev/null", "w", stdout); - freopen("/dev/null", "w", stderr); - - pid = fork(); - if (pid == 0) { - // At this point we are executing as the 2nd child process - // Replace this clone with the process we'd like to start - char * const args[] = { strdup(VMCHOOSER_SESSION_START_SCRIPT), nullptr }; - execv(VMCHOOSER_SESSION_START_SCRIPT, args); - } - _exit(0); // Dont use exit hooks - } - // Wait and cleanup the intermediate process - waitpid(pid, nullptr, 0); + UserConfig::addLastSession(s->uuid().isEmpty() ? s->shortDescription() : s->uuid()); + UserConfig::setLastTab(activeTab_); + UserConfig::setNewsHelpOpen(!ui->helpBox->isHidden()); + UserConfig::sync(); + + QProcess *process = nullptr; + if (QFile::exists(SESSION_START_SCRIPT)) { + // Companion script + process = new QProcess; + QProcessEnvironment env = QProcessEnvironment::systemEnvironment(); + env.insert("SESSION_NAME", s->shortDescription()); + env.insert("SESSION_UUID", s->uuid()); + env.insert("SESSION_CMD", s->execCommand()); + if (s->type() == Session::VSESSION) { + env.insert("SESSION_TYPE", "VSESSION"); + } else if (s->type() == Session::XSESSION) { + env.insert("SESSION_TYPE", "XSESSION"); } - UserConfig::addLastSession(s->uuid().isEmpty() ? s->shortDescription() : s->uuid()); - UserConfig::setLastTab(activeTab_); - UserConfig::setNewsHelpOpen(!ui->helpBox->isHidden()); - centerTimer_->stop(); // Stop the auto-center/auto-quit timer, so we don't kill the session :> - setVisible(false); - // Stopping the window manager has to be done after hiding our window, - // or it might still be shown in some Xsessions (e.g. gnome-shell). - WindowManager::stopOwnInstance(); - } else { - QMessageBox::critical( - this, trUtf8("vmchooser"), - trUtf8("Vmchooser failed to run the selected session!")); + process->setProcessEnvironment(env); + process->start(SESSION_START_SCRIPT); + process->closeReadChannel(QProcess::StandardError); + process->closeReadChannel(QProcess::StandardOutput); + process->closeWriteChannel(); } + + // Run session + s->run(); + // Should not return on success + if (process != nullptr) { + process->terminate(); + } + QMessageBox::critical( + this, trUtf8("vmchooser"), + trUtf8("Vmchooser failed to run the selected session!")); + QApplication::instance()->quit(); } void Dialog::on_treeView_expanded(const QModelIndex& index) { diff --git a/src/session.h b/src/session.h index 01f8ad5..c3f758c 100644 --- a/src/session.h +++ b/src/session.h @@ -39,7 +39,8 @@ class Session { virtual QString shortDescription() const = 0; virtual QString description() const = 0; virtual QIcon icon() const = 0; - virtual bool run() const = 0; + virtual bool prepareRun() const = 0; + virtual void run() const = 0; virtual QString execCommand() const { return QString(); } virtual QString uuid() const { return QString(); } virtual int type() const = 0; diff --git a/src/userconfig.cpp b/src/userconfig.cpp index 7bc83fe..31ab9e9 100644 --- a/src/userconfig.cpp +++ b/src/userconfig.cpp @@ -90,3 +90,10 @@ bool UserConfig::isNewsHelpOpen() init(); return settings->value(KEY_NEWS_OPEN).toBool(); } + +void UserConfig::sync() +{ + if (settings != nullptr) { + settings->sync(); + } +} diff --git a/src/userconfig.h b/src/userconfig.h index 213c4c2..4fdf638 100644 --- a/src/userconfig.h +++ b/src/userconfig.h @@ -14,6 +14,7 @@ public: static void setLastTab(int tab); static void setLastNewsTime(uint t); static void setNewsHelpOpen(bool b); + static void sync(); private: UserConfig() {} diff --git a/src/vsession.cpp b/src/vsession.cpp index 79ef929..b61c759 100644 --- a/src/vsession.cpp +++ b/src/vsession.cpp @@ -21,8 +21,6 @@ #include "userldapdata.h" #include "virtualizer.h" -static QProcess _process; - static const QString VMWARE("vmware"); static const QString VIRTUALBOX("virtualbox"); @@ -65,6 +63,8 @@ static const QStringList directOsNames( static const QRegularExpression cleanNameRegex("[^a-z0-9]"); +QString tmpFileName; + bool VSession::init(const QDomElement& xml) { QDomElement settingsNode = this->doc_.createElement("settings"); this->doc_.appendChild(settingsNode); @@ -238,44 +238,48 @@ int VSession::priority() const { return prio; } -bool VSession::run() const { - if (_process.state() != QProcess::NotRunning) { - qDebug() << "Cannot start vsession while old one is still running"; +bool VSession::prepareRun() const { + if (!canRun(true)) return false; - } - if (g_debugMode) { - qDebug() << "Sarting session " << shortDescription() << " ..."; - } - - if (g_noVtx && needsVtx()) { - QMessageBox::warning(nullptr, QObject::trUtf8("Warning"), - QObject::trUtf8("The selected session is based on a 64 bit operating system," - " but this computer doesn't seem to support this (VT-x/AMD-V not" - " supported by CPU, or disabled in BIOS). You will probably get an" - " error message while the virtualizer is initializing.")); - } - - QString command = getAttribute(QStringLiteral("command")); - if (!command.isEmpty()) { - return QProcess::startDetached(command); - } - // write xml to temporary file QTemporaryFile tmpfile(QDir::tempPath() + QStringLiteral("/vmchooser-XXXXXX.xml")); if (!tmpfile.open() || tmpfile.write(this->toXml().toUtf8()) == -1) { qDebug() << "Error writing xml to file" << tmpfile.fileName(); + QMessageBox::critical( + nullptr, QObject::trUtf8("vmchooser"), + QObject::trUtf8("Error writing temporary XML file for run-virt")); return false; } - // Docs say we should call fileName() before closing to prevent deletion - QString tmpFileName = tmpfile.fileName(); + if (!tmpFileName.isEmpty()) { + QFile(tmpFileName).remove(); + } + // Docs say we should call fileName() before closing, to prevent deletion + tmpFileName = tmpfile.fileName(); tmpfile.setAutoRemove(false); tmpfile.close(); + if (g_noVtx && needsVtx()) { + QMessageBox::StandardButton ret = QMessageBox::question(nullptr, QObject::tr("Warning"), + QObject::tr("The selected session is based on a 64 bit operating system," + " but this computer doesn't seem to support this (VT-x/AMD-V not" + " supported by CPU, or disabled in BIOS). You will probably get an" + " error message while the virtualizer is initializing.\n\n" + "Do you still want to try to start this VM?"), + QMessageBox::Yes | QMessageBox::No, QMessageBox::No); + if (ret != QMessageBox::Yes) + return false; + } + return true; +} - QObject::connect(&_process, QOverload::of(&QProcess::finished), QApplication::instance(), &QCoreApplication::quit); - _process.start(Config::get(Config::RUNSCRIPT), QStringList(tmpFileName)); - _process.waitForStarted(10); - return _process.state() == QProcess::Starting || _process.state() == QProcess::Running; +void VSession::run() const { + if (g_debugMode) { + qDebug() << "Sarting session " << shortDescription() << " ..."; + } + QByteArray fn = tmpFileName.toUtf8(); + QByteArray script = Config::get(Config::RUNSCRIPT).toUtf8(); + char *argv[3] = { script.data(), fn.data(), nullptr }; + execv(script.data(), argv); } int VSession::type() const { diff --git a/src/vsession.h b/src/vsession.h index 69ecf13..e571805 100644 --- a/src/vsession.h +++ b/src/vsession.h @@ -77,7 +77,9 @@ class VSession : public Session { QString toXml() const; - bool run() const; + bool prepareRun() const; + + void run() const; int type() const; diff --git a/src/windowmanager.cpp b/src/windowmanager.cpp index 2505c89..c05e234 100644 --- a/src/windowmanager.cpp +++ b/src/windowmanager.cpp @@ -38,12 +38,16 @@ void ensureRunning() }); } -void stopOwnInstance() +void stopOwnInstance(bool waitSync) { if (wm.state() == QProcess::Running) { qDebug() << "- Politely terminating spawned WM"; - wm.terminate(); - QTimer::singleShot(500, killInstance); + if (waitSync) { + killInstance(); + } else { + wm.terminate(); + QTimer::singleShot(500, killInstance); + } } } @@ -51,7 +55,9 @@ void killInstance() { if (wm.state() == QProcess::Running) { wm.terminate(); - QThread::msleep(50); + } + for (int i = 0; i < 50 && wm.state() == QProcess::Running; ++i) { + QThread::msleep(10); } if (wm.state() == QProcess::Running) { qDebug() << "- Forcefully killing spawned WM"; diff --git a/src/windowmanager.h b/src/windowmanager.h index c89f983..26d5606 100644 --- a/src/windowmanager.h +++ b/src/windowmanager.h @@ -15,7 +15,7 @@ void ensureRunning(); * terminate it, otherwise leave the current * one running. */ -void stopOwnInstance(); +void stopOwnInstance(bool waitSync); } diff --git a/src/xsession.cpp b/src/xsession.cpp index 05f4d38..5db3b11 100644 --- a/src/xsession.cpp +++ b/src/xsession.cpp @@ -2,8 +2,8 @@ #include #include #include -#include #include +#include "unistd.h" #include "xsession.h" #include "globals.h" @@ -18,10 +18,11 @@ struct DisplayConfig { static QList< DisplayConfig > priorityList; -void XSession::init(const QString& name, const QString& exec, +void XSession::init(const QString& name, const QString& exec, const QString& tryExec, const QString& comment, const QString& icon) { this->name_ = name; this->exec_ = exec; + this->tryExec_ = tryExec; this->comment_ = comment; this->icon_ = icon; this->priority_ = 0; @@ -43,6 +44,7 @@ bool XSession::init(const QString& filename) { return false; } QString exec(settings.value("Exec").toString()); + QString tryExec(settings.value("TryExec").toString()); QString locale(QLocale::system().name()); QString language(locale.split("_").at(0)); QString defaultName(""); @@ -100,11 +102,10 @@ bool XSession::init(const QString& filename) { this->name_ = name; this->exec_ = exec; + this->tryExec_ = tryExec; this->comment_ = comment; this->icon_ = icon; - _process = new QProcess(); - return true; } @@ -113,7 +114,12 @@ bool XSession::isActive() const { } QString XSession::checkCanRunInternal() const { - QString exe = this->exec_.left(this->exec_.indexOf(' ')); // -1 means entire string + QString exe; + if (this->tryExec_.isEmpty()) { + exe = this->exec_.left(this->exec_.indexOf(' ')); // -1 means entire string + } else { + exe = this->tryExec_; + } QFileInfo fi(exe); if (!fi.isAbsolute()) { if (!QStandardPaths::findExecutable(exe).isEmpty()) @@ -156,10 +162,21 @@ QIcon XSession::icon() const { return retIcon; } -bool XSession::run() const { - _process->start(this->exec_); - QObject::connect(_process, QOverload::of(&QProcess::finished), QApplication::instance(), &QCoreApplication::quit); - return _process->state() == QProcess::Starting || _process->state() == QProcess::Running; +bool XSession::prepareRun() const { + return canRun(true); +} + +void XSession::run() const { + QString command; + QString firstToken = this->exec_.left(this->exec_.indexOf(' ')); + if (!firstToken.contains('=')) { // First token doesn't appear to be environment modification, try to prefix with exec + command += "exec "; + } + command += exec_; + qDebug() << "Running via /bin/sh:" << command; + QByteArray cmdbin = command.toUtf8(); + char *argv[4] = { "/bin/sh", "-c", cmdbin.data(), nullptr }; + execv("/bin/sh", argv); } int XSession::type() const { diff --git a/src/xsession.h b/src/xsession.h index e014e9e..7b9135b 100644 --- a/src/xsession.h +++ b/src/xsession.h @@ -11,7 +11,7 @@ class XSession : public Session { public: - void init(const QString& name, const QString& exec, + void init(const QString& name, const QString& exec, const QString& tryExec, const QString& comment, const QString& icon); bool init(const QString& filename); @@ -40,7 +40,9 @@ class XSession : public Session { QIcon icon() const; - bool run() const; + bool prepareRun() const; + + void run() const; int type() const; @@ -56,12 +58,11 @@ protected: private: QString name_; QString exec_; + QString tryExec_; QString comment_; QString icon_; int priority_; - QProcess *_process; - static void loadXSessionsConfig(); static inline int find(QChar c, int start, const QString& line) -- cgit v1.2.3-55-g7522