From 80c8d324b335753d5b1758f29775b844904bb2c6 Mon Sep 17 00:00:00 2001 From: Joerg Bornemann Date: Wed, 29 Apr 2015 14:36:24 +0200 Subject: take process name into account for QLockFile's pid clash resolution To cover the situation that the process ID got reused, the current process name is compared to the name of the process that corresponds to the process ID from the lock file. If the process names differ, the lock file is considered stale. [ChangeLog][QtCore][QLockFile] Detection of stale lock files got more robust and takes the name of the process that belongs to the stored PID into account. Task-number: QTBUG-45497 Change-Id: Ic3c0d7e066435451203e77b9b9ce2d70bfb9c570 Reviewed-by: Friedemann Kleint Reviewed-by: David Faure --- src/corelib/io/qlockfile.cpp | 12 ++++-- src/corelib/io/qlockfile_p.h | 1 + src/corelib/io/qlockfile_unix.cpp | 48 +++++++++++++++++++++++ src/corelib/io/qlockfile_win.cpp | 45 +++++++++++++++++++++ tests/auto/corelib/io/qlockfile/tst_qlockfile.cpp | 48 +++++++++++++++++++++++ 5 files changed, 150 insertions(+), 4 deletions(-) Index: qtbase-opensource-src-5.4.2+dfsg/src/corelib/io/qlockfile.cpp =================================================================== --- qtbase-opensource-src-5.4.2+dfsg.orig/src/corelib/io/qlockfile.cpp +++ qtbase-opensource-src-5.4.2+dfsg/src/corelib/io/qlockfile.cpp @@ -1,6 +1,7 @@ /**************************************************************************** ** ** Copyright (C) 2013 David Faure +** Copyright (C) 2015 The Qt Company Ltd. ** Contact: http://www.qt-project.org/legal ** ** This file is part of the QtCore module of the Qt Toolkit. @@ -66,9 +67,12 @@ QT_BEGIN_NAMESPACE If the process holding the lock crashes, the lock file stays on disk and can prevent any other process from accessing the shared resource, ever. For this reason, QLockFile - tries to detect such a "stale" lock file, based on the process ID written into the file, - and (in case that process ID got reused meanwhile), on the last modification time of - the lock file (30s by default, for the use case of a short-lived operation). + tries to detect such a "stale" lock file, based on the process ID written into the file. + To cover the situation that the process ID got reused meanwhile, the current process name is + compared to the name of the process that corresponds to the process ID from the lock file. + If the process names differ, the lock file is considered stale. + Additionally, the last modification time of the lock file (30s by default, for the use case of a + short-lived operation) is taken into account. If the lock file is found to be stale, it will be deleted. For the use case of protecting a resource over a long time, you should therefore call @@ -122,7 +126,7 @@ QLockFile::~QLockFile() The value of \a staleLockTime is used by lock() and tryLock() in order to determine when an existing lock file is considered stale, i.e. left over by a crashed process. This is useful for the case where the PID got reused - meanwhile, so the only way to detect a stale lock file is by the fact that + meanwhile, so one way to detect a stale lock file is by the fact that it has been around for a long time. \sa staleLockTime() Index: qtbase-opensource-src-5.4.2+dfsg/src/corelib/io/qlockfile_p.h =================================================================== --- qtbase-opensource-src-5.4.2+dfsg.orig/src/corelib/io/qlockfile_p.h +++ qtbase-opensource-src-5.4.2+dfsg/src/corelib/io/qlockfile_p.h @@ -75,6 +75,7 @@ public: // Returns \c true if the lock belongs to dead PID, or is old. // The attempt to delete it will tell us if it was really stale or not, though. bool isApparentlyStale() const; + static QString processNameByPid(qint64 pid); #ifdef Q_OS_UNIX static int checkFcntlWorksAfterFlock(); Index: qtbase-opensource-src-5.4.2+dfsg/src/corelib/io/qlockfile_unix.cpp =================================================================== --- qtbase-opensource-src-5.4.2+dfsg.orig/src/corelib/io/qlockfile_unix.cpp +++ qtbase-opensource-src-5.4.2+dfsg/src/corelib/io/qlockfile_unix.cpp @@ -1,6 +1,7 @@ /**************************************************************************** ** ** Copyright (C) 2013 David Faure +** Copyright (C) 2015 The Qt Company Ltd. ** Contact: http://www.qt-project.org/legal ** ** This file is part of the QtCore module of the Qt Toolkit. @@ -48,6 +49,15 @@ #include // kill #include // gethostname +#if defined(Q_OS_OSX) +# include +#elif defined(Q_OS_LINUX) +# include +# include +#elif defined(Q_OS_BSD4) && !defined(Q_OS_IOS) +# include +#endif + QT_BEGIN_NAMESPACE static QByteArray localHostName() // from QHostInfo::localHostName(), modified to return a QByteArray @@ -185,11 +195,49 @@ bool QLockFilePrivate::isApparentlyStale if (hostname.isEmpty() || hostname == QString::fromLocal8Bit(localHostName())) { if (::kill(pid, 0) == -1 && errno == ESRCH) return true; // PID doesn't exist anymore + const QString processName = processNameByPid(pid); + if (!processName.isEmpty()) { + QFileInfo fi(appname); + if (fi.isSymLink()) + fi.setFile(fi.symLinkTarget()); + if (processName != fi.fileName()) + return true; // PID got reused by a different application. + } } const qint64 age = QFileInfo(fileName).lastModified().msecsTo(QDateTime::currentDateTime()); return staleLockTime > 0 && age > staleLockTime; } +QString QLockFilePrivate::processNameByPid(qint64 pid) +{ +#if defined(Q_OS_OSX) + char name[1024]; + proc_name(pid, name, sizeof(name) / sizeof(char)); + return QString::fromUtf8(name); +#elif defined(Q_OS_LINUX) + if (!QFile::exists(QStringLiteral("/proc/version"))) + return QString(); + char exePath[64]; + char buf[PATH_MAX]; + memset(buf, 0, sizeof(buf)); + sprintf(exePath, "/proc/%lld/exe", pid); + if (readlink(exePath, buf, sizeof(buf)) < 0) { + // The pid is gone. Return some invalid process name to fail the test. + return QStringLiteral("/ERROR/"); + } + return QFileInfo(QString::fromUtf8(buf)).fileName(); +#elif defined(Q_OS_BSD4) && !defined(Q_OS_IOS) + kinfo_proc *proc = kinfo_getproc(pid); + if (!proc) + return QString(); + QString name = QString::fromUtf8(proc->ki_comm); + free(proc); + return name; +#else + return QString(); +#endif +} + void QLockFile::unlock() { Q_D(QLockFile); Index: qtbase-opensource-src-5.4.2+dfsg/src/corelib/io/qlockfile_win.cpp =================================================================== --- qtbase-opensource-src-5.4.2+dfsg.orig/src/corelib/io/qlockfile_win.cpp +++ qtbase-opensource-src-5.4.2+dfsg/src/corelib/io/qlockfile_win.cpp @@ -1,6 +1,7 @@ /**************************************************************************** ** ** Copyright (C) 2013 David Faure +** Copyright (C) 2015 The Qt Company Ltd. ** Contact: http://www.qt-project.org/legal ** ** This file is part of the QtCore module of the Qt Toolkit. @@ -129,6 +130,9 @@ bool QLockFilePrivate::isApparentlyStale ::CloseHandle(procHandle); if (dwR == WAIT_TIMEOUT) return true; + const QString processName = processNameByPid(pid); + if (!processName.isEmpty() && processName != appname) + return true; // PID got reused by a different application. } #endif // !Q_OS_WINRT const qint64 age = QFileInfo(fileName).lastModified().msecsTo(QDateTime::currentDateTime()); @@ -136,6 +140,47 @@ bool QLockFilePrivate::isApparentlyStale return staleLockTime > 0 && age > staleLockTime; } +QString QLockFilePrivate::processNameByPid(qint64 pid) +{ +#if !defined(Q_OS_WINRT) && !defined(Q_OS_WINCE) + typedef DWORD (WINAPI *GetModuleFileNameExFunc)(HANDLE, HMODULE, LPTSTR, DWORD); + + HMODULE hPsapi = LoadLibraryA("psapi"); + if (!hPsapi) + return QString(); + + GetModuleFileNameExFunc qGetModuleFileNameEx + = (GetModuleFileNameExFunc)GetProcAddress(hPsapi, "GetModuleFileNameExW"); + if (!qGetModuleFileNameEx) { + FreeLibrary(hPsapi); + return QString(); + } + + HANDLE hProcess = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, DWORD(pid)); + if (!hProcess) { + FreeLibrary(hPsapi); + return QString(); + } + wchar_t buf[MAX_PATH]; + const DWORD length = qGetModuleFileNameEx(hProcess, NULL, buf, sizeof(buf) / sizeof(wchar_t)); + CloseHandle(hProcess); + FreeLibrary(hPsapi); + if (!length) + return QString(); + QString name = QString::fromWCharArray(buf, length); + int i = name.lastIndexOf(QLatin1Char('\\')); + if (i >= 0) + name.remove(0, i + 1); + i = name.lastIndexOf(QLatin1Char('.')); + if (i >= 0) + name.truncate(i); + return name; +#else + Q_UNUSED(pid); + return QString(); +#endif +} + void QLockFile::unlock() { Q_D(QLockFile); Index: qtbase-opensource-src-5.4.2+dfsg/tests/auto/corelib/io/qlockfile/tst_qlockfile.cpp =================================================================== --- qtbase-opensource-src-5.4.2+dfsg.orig/tests/auto/corelib/io/qlockfile/tst_qlockfile.cpp +++ qtbase-opensource-src-5.4.2+dfsg/tests/auto/corelib/io/qlockfile/tst_qlockfile.cpp @@ -54,11 +54,15 @@ private slots: void waitForLock(); void staleLockFromCrashedProcess_data(); void staleLockFromCrashedProcess(); + void staleLockFromCrashedProcessReusedPid(); void staleShortLockFromBusyProcess(); void staleLongLockFromBusyProcess(); void staleLockRace(); void noPermissions(); +private: + static bool overwritePidInLockFile(const QString &filePath, qint64 pid); + public: QString m_helperApp; QTemporaryDir dir; @@ -273,6 +277,30 @@ void tst_QLockFile::staleLockFromCrashed #endif // !QT_NO_PROCESS } +void tst_QLockFile::staleLockFromCrashedProcessReusedPid() +{ +#if defined(QT_NO_PROCESS) + QSKIP("This test requires QProcess support"); +#elif defined(Q_OS_WINRT) || defined(Q_OS_WINCE) || defined(Q_OS_IOS) + QSKIP("We cannot retrieve information about other processes on this platform."); +#else + const QString fileName = dir.path() + "/staleLockFromCrashedProcessReusedPid"; + + int ret = QProcess::execute(m_helperApp, QStringList() << fileName << "-crash"); + QCOMPARE(ret, int(QLockFile::NoError)); + QVERIFY(QFile::exists(fileName)); + QVERIFY(overwritePidInLockFile(fileName, QCoreApplication::applicationPid())); + + QLockFile secondLock(fileName); + qint64 pid = 0; + secondLock.getLockInfo(&pid, 0, 0); + QCOMPARE(pid, QCoreApplication::applicationPid()); + secondLock.setStaleLockTime(0); + QVERIFY(secondLock.tryLock()); + QCOMPARE(int(secondLock.error()), int(QLockFile::NoError)); +#endif // !QT_NO_PROCESS +} + void tst_QLockFile::staleShortLockFromBusyProcess() { #ifdef QT_NO_PROCESS @@ -433,5 +461,25 @@ void tst_QLockFile::noPermissions() QCOMPARE(int(lockFile.error()), int(QLockFile::PermissionError)); } +bool tst_QLockFile::overwritePidInLockFile(const QString &filePath, qint64 pid) +{ + QFile f(filePath); + if (!f.open(QFile::ReadWrite)) { + qWarning("Cannot open %s.", qPrintable(filePath)); + return false; + } + QByteArray buf = f.readAll(); + int i = buf.indexOf('\n'); + if (i < 0) { + qWarning("Unexpected lockfile content."); + return false; + } + buf.remove(0, i); + buf.prepend(QByteArray::number(pid)); + f.seek(0); + f.resize(buf.size()); + return f.write(buf) == buf.size(); +} + QTEST_MAIN(tst_QLockFile) #include "tst_qlockfile.moc"