Changeset View
Standalone View
src/kdeinitinterface.cpp
| Show All 35 Lines | |||||
| 36 | { | 36 | { | ||
| 37 | QDBusConnectionInterface *dbusDaemon = KDBusConnectionPool::threadConnection().interface(); | 37 | QDBusConnectionInterface *dbusDaemon = KDBusConnectionPool::threadConnection().interface(); | ||
| 38 | if (dbusDaemon->isServiceRegistered(QStringLiteral("org.kde.klauncher5"))) { | 38 | if (dbusDaemon->isServiceRegistered(QStringLiteral("org.kde.klauncher5"))) { | ||
| 39 | return; | 39 | return; | ||
| 40 | } | 40 | } | ||
| 41 | qCDebug(KDBUSADDONS_LOG) << "klauncher not running... launching kdeinit"; | 41 | qCDebug(KDBUSADDONS_LOG) << "klauncher not running... launching kdeinit"; | ||
| 42 | 42 | | |||
| 43 | QLockFile lock(QDir::tempPath() + QLatin1Char('/') + QLatin1String("startkdeinitlock")); | 43 | QLockFile lock(QDir::tempPath() + QLatin1Char('/') + QLatin1String("startkdeinitlock")); | ||
| 44 | // If we can't get the lock, then someone else is already in the process of starting kdeinit. | ||||
| 44 | if (!lock.tryLock()) { | 45 | if (!lock.tryLock()) { | ||
thiago: This line doesn't need changing. You're getting a tryLock() failure because the lock file… | |||||
| 45 | lock.lock(); | 46 | // Wait for that to happen, by locking again 30 seconds max. | ||
| 47 | if (!lock.tryLock(30000)) | ||||
| 48 | { | ||||
| 49 | qCWarning(KDBUSADDONS_LOG) << "'kdeinit5' is taking more than 30 seconds to start."; | ||||
| 50 | return; | ||||
| 51 | } | ||||
| 52 | // Check that the DBus name is up, i.e. the other process did manage to do it successfully. | ||||
The problem is here. So we failed to lock, then we try again to lock, forever. Why is this code doing that? thiago: The problem is here. So we failed to lock, then we try again to lock, forever. Why is this code… | |||||
| 46 | if (dbusDaemon->isServiceRegistered(QStringLiteral("org.kde.klauncher5"))) { | 53 | if (dbusDaemon->isServiceRegistered(QStringLiteral("org.kde.klauncher5"))) { | ||
| 47 | return; // whoever held the lock has already started it | 54 | return; | ||
| 48 | } | 55 | } | ||
| 49 | } | 56 | } | ||
| 50 | // Try to launch kdeinit. | 57 | // Try to launch kdeinit. | ||
| 51 | QString srv = QStandardPaths::findExecutable(QStringLiteral("kdeinit5")); | 58 | QString srv = QStandardPaths::findExecutable(QStringLiteral("kdeinit5")); | ||
| 52 | // If not found in system paths, search other paths | 59 | // If not found in system paths, search other paths | ||
| 53 | if (srv.isEmpty()) { | 60 | if (srv.isEmpty()) { | ||
| 54 | const QStringList searchPaths = QStringList() | 61 | const QStringList searchPaths = QStringList() | ||
| 55 | << QCoreApplication::applicationDirPath() // then look where our application binary is located | 62 | << QCoreApplication::applicationDirPath() // then look where our application binary is located | ||
| 56 | << QLibraryInfo::location(QLibraryInfo::BinariesPath); // look where exec path is (can be set in qt.conf) | 63 | << QLibraryInfo::location(QLibraryInfo::BinariesPath); // look where exec path is (can be set in qt.conf) | ||
| 57 | srv = QStandardPaths::findExecutable(QStringLiteral("kdeinit5"), searchPaths); | 64 | srv = QStandardPaths::findExecutable(QStringLiteral("kdeinit5"), searchPaths); | ||
| 58 | if (srv.isEmpty()) { | 65 | if (srv.isEmpty()) { | ||
| 59 | qCWarning(KDBUSADDONS_LOG) << "Can not find 'kdeinit5' executable at " << qgetenv("PATH") << searchPaths.join(QStringLiteral(", ")); | 66 | qCWarning(KDBUSADDONS_LOG) << "Can not find 'kdeinit5' executable at " << qgetenv("PATH") << searchPaths.join(QStringLiteral(", ")); | ||
| 60 | return; | 67 | return; | ||
| 61 | } | 68 | } | ||
| 62 | } | 69 | } | ||
| 63 | 70 | | |||
| 64 | QStringList args; | 71 | QStringList args; | ||
| 65 | #ifndef Q_OS_WIN | 72 | #ifndef Q_OS_WIN | ||
| 66 | args += QStringLiteral("--suicide"); | 73 | args += QStringLiteral("--suicide"); | ||
| 67 | #endif | 74 | #endif | ||
| 75 | // NOTE: kdeinit5 is supposed to finish quickly, certainly in less than 30 seconds. | ||||
| 68 | QProcess::execute(srv, args); | 76 | QProcess::execute(srv, args); | ||
| 69 | } | 77 | } | ||
But that means the other processes coming into this method, will either succeed in tryLock() and will run yet another kdeinit instance (your system is slow and you're bombarding it with kdeinit processes trying to start at the same time?), If the bug is that kdeinit takes forever to start (not just a long time) and QProcess::execute never returns, then that's the actual bug. This proposed change is just a workaround which potentially makes things worse (10 kdeinit processes attempting to start at the same time). BTW ~QProcess would kill kdeinit, what you meant was startDetached. But all of the above is the reasoning against startDetached, actually ;) dfaure: But that means the other processes coming into this method, will either succeed in tryLock()… | |||||
| 70 | 78 | | |||
This line doesn't need changing. You're getting a tryLock() failure because the lock file already exists. Adding a 5 second timeout is not going to change that.