summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThiago Macieira <thiago.macieira@intel.com>2017-09-29 09:59:18 -0700
committerThiago Macieira <thiago.macieira@intel.com>2017-11-14 15:54:56 +0000
commit772863355a0cf57a49e93608790dfd17c8fd82da (patch)
tree5bece011b9465288e23c941563a3c887c5536c98
parent68cad0ef99b071cf03b8477d828f732485fca16b (diff)
QLockFile/Unix: save the boot and machine IDs in the lock file too
This allows us to make sure that the PID we read is from the same boot as we are right now running. The collision could happen on embedded systems where the boot sequence is fixed, so all the same processes would have the exact same PIDs after reboot as they did before. [ChangeLog][QtCore][QLockFile] QLockFile can now properly conclude that a lock file from a previous boot of the same device is stale and can be removed. This is implemented only for Linux and Apple operating systems. Task-number: QTBUG-63425 Change-Id: I0b48fc8e90304e0dacc3fffd14e8e3a197211788 Reviewed-by: Kai Koehne <kai.koehne@qt.io>
-rw-r--r--src/corelib/io/qlockfile.cpp31
-rw-r--r--src/corelib/io/qlockfile_p.h16
-rw-r--r--src/corelib/io/qlockfile_unix.cpp2
-rw-r--r--src/corelib/io/qlockfile_win.cpp4
-rw-r--r--tests/auto/corelib/io/qlockfile/tst_qlockfile.cpp107
-rw-r--r--tests/auto/corelib/io/qlockfile/tst_qlockfile.pro2
6 files changed, 148 insertions, 14 deletions
diff --git a/src/corelib/io/qlockfile.cpp b/src/corelib/io/qlockfile.cpp
index 129cf01b63..aa84ce6bc1 100644
--- a/src/corelib/io/qlockfile.cpp
+++ b/src/corelib/io/qlockfile.cpp
@@ -56,6 +56,8 @@ struct LockFileInfo
qint64 pid;
QString appname;
QString hostname;
+ QByteArray hostid;
+ QByteArray bootid;
};
}
@@ -331,13 +333,15 @@ QByteArray QLockFilePrivate::lockFileContents() const
// Use operator% from the fast builder to avoid multiple memory allocations.
return QByteArray::number(QCoreApplication::applicationPid()) % '\n'
% processNameByPid(QCoreApplication::applicationPid()).toUtf8() % '\n'
- % machineName().toUtf8() % '\n';
+ % machineName().toUtf8() % '\n'
+ % QSysInfo::machineUniqueId() % '\n'
+ % QSysInfo::bootUniqueId() % '\n';
}
static bool getLockInfo_helper(const QString &fileName, LockFileInfo *info)
{
QFile reader(fileName);
- if (!reader.open(QIODevice::ReadOnly))
+ if (!reader.open(QIODevice::ReadOnly | QIODevice::Text))
return false;
QByteArray pidLine = reader.readLine();
@@ -349,9 +353,17 @@ static bool getLockInfo_helper(const QString &fileName, LockFileInfo *info)
QByteArray hostNameLine = reader.readLine();
hostNameLine.chop(1);
+ // prior to Qt 5.10, only the lines above were recorded
+ QByteArray hostId = reader.readLine();
+ hostId.chop(1);
+ QByteArray bootId = reader.readLine();
+ bootId.chop(1);
+
bool ok;
info->appname = QString::fromUtf8(appNameLine);
info->hostname = QString::fromUtf8(hostNameLine);
+ info->hostid = hostId;
+ info->bootid = bootId;
info->pid = pidLine.toLongLong(&ok);
return ok && info->pid > 0;
}
@@ -360,7 +372,20 @@ bool QLockFilePrivate::isApparentlyStale() const
{
LockFileInfo info;
if (getLockInfo_helper(fileName, &info)) {
- if (info.hostname.isEmpty() || info.hostname == machineName()) {
+ bool sameHost = info.hostname.isEmpty() || info.hostname == machineName();
+ if (!info.hostid.isEmpty()) {
+ // Override with the host ID, if we know it.
+ QByteArray ourHostId = QSysInfo::machineUniqueId();
+ if (!ourHostId.isEmpty())
+ sameHost = (ourHostId == info.hostid);
+ }
+
+ if (sameHost) {
+ if (!info.bootid.isEmpty()) {
+ // If we've rebooted, then the lock is definitely stale.
+ if (info.bootid != QSysInfo::bootUniqueId())
+ return true;
+ }
if (!isProcessRunning(info.pid, info.appname))
return true;
}
diff --git a/src/corelib/io/qlockfile_p.h b/src/corelib/io/qlockfile_p.h
index b41dfb38ad..5b69347206 100644
--- a/src/corelib/io/qlockfile_p.h
+++ b/src/corelib/io/qlockfile_p.h
@@ -55,7 +55,10 @@
#include <QtCore/qlockfile.h>
#include <QtCore/qfile.h>
+#include <qplatformdefs.h>
+
#ifdef Q_OS_WIN
+#include <io.h>
#include <qt_windows.h>
#endif
@@ -96,6 +99,19 @@ public:
int staleLockTime; // "int milliseconds" is big enough for 24 days
QLockFile::LockError lockError;
bool isLocked;
+
+ static int getLockFileHandle(QLockFile *f)
+ {
+ int fd;
+#ifdef Q_OS_WIN
+ // Use of this function on Windows WILL leak a file descriptor.
+ fd = _open_osfhandle(intptr_t(f->d_func()->fileHandle), 0);
+#else
+ fd = f->d_func()->fileHandle;
+#endif
+ QT_LSEEK(fd, 0, SEEK_SET);
+ return fd;
+ }
};
QT_END_NAMESPACE
diff --git a/src/corelib/io/qlockfile_unix.cpp b/src/corelib/io/qlockfile_unix.cpp
index fc01f83e80..418b7d22ba 100644
--- a/src/corelib/io/qlockfile_unix.cpp
+++ b/src/corelib/io/qlockfile_unix.cpp
@@ -147,7 +147,7 @@ static bool setNativeLocks(int fd)
QLockFile::LockError QLockFilePrivate::tryLock_sys()
{
const QByteArray lockFileName = QFile::encodeName(fileName);
- const int fd = qt_safe_open(lockFileName.constData(), O_WRONLY | O_CREAT | O_EXCL, 0666);
+ const int fd = qt_safe_open(lockFileName.constData(), O_RDWR | O_CREAT | O_EXCL, 0666);
if (fd < 0) {
switch (errno) {
case EEXIST:
diff --git a/src/corelib/io/qlockfile_win.cpp b/src/corelib/io/qlockfile_win.cpp
index de64ec0432..6b8028460c 100644
--- a/src/corelib/io/qlockfile_win.cpp
+++ b/src/corelib/io/qlockfile_win.cpp
@@ -68,7 +68,7 @@ QLockFile::LockError QLockFilePrivate::tryLock_sys()
#ifndef Q_OS_WINRT
SECURITY_ATTRIBUTES securityAtts = { sizeof(SECURITY_ATTRIBUTES), NULL, FALSE };
HANDLE fh = CreateFile((const wchar_t*)fileEntry.nativeFilePath().utf16(),
- GENERIC_WRITE,
+ GENERIC_READ | GENERIC_WRITE,
dwShareMode,
&securityAtts,
CREATE_NEW, // error if already exists
@@ -76,7 +76,7 @@ QLockFile::LockError QLockFilePrivate::tryLock_sys()
NULL);
#else // !Q_OS_WINRT
HANDLE fh = CreateFile2((const wchar_t*)fileEntry.nativeFilePath().utf16(),
- GENERIC_WRITE,
+ GENERIC_READ | GENERIC_WRITE,
dwShareMode,
CREATE_NEW, // error if already exists
NULL);
diff --git a/tests/auto/corelib/io/qlockfile/tst_qlockfile.cpp b/tests/auto/corelib/io/qlockfile/tst_qlockfile.cpp
index 835c4a2778..fc7ab70d41 100644
--- a/tests/auto/corelib/io/qlockfile/tst_qlockfile.cpp
+++ b/tests/auto/corelib/io/qlockfile/tst_qlockfile.cpp
@@ -39,6 +39,8 @@
# include <qt_windows.h>
#endif
+#include <private/qlockfile_p.h> // for getLockFileHandle()
+
class tst_QLockFile : public QObject
{
Q_OBJECT
@@ -61,8 +63,12 @@ private slots:
void noPermissionsWindows();
void corruptedLockFile();
void corruptedLockFileInTheFuture();
+ void hostnameChange();
+ void differentMachines();
+ void reboot();
private:
+ static bool overwriteLineInLockFile(QFile &f, int line, const QString &newLine);
static bool overwritePidInLockFile(const QString &filePath, qint64 pid);
public:
@@ -295,7 +301,7 @@ void tst_QLockFile::staleLockFromCrashedProcessReusedPid()
QLockFile secondLock(fileName);
qint64 pid = 0;
- secondLock.getLockInfo(&pid, 0, 0);
+ QVERIFY(secondLock.getLockInfo(&pid, 0, 0));
QCOMPARE(pid, QCoreApplication::applicationPid());
secondLock.setStaleLockTime(0);
QVERIFY(secondLock.tryLock());
@@ -549,22 +555,109 @@ void tst_QLockFile::corruptedLockFileInTheFuture()
#endif
}
+void tst_QLockFile::hostnameChange()
+{
+ const QByteArray hostid = QSysInfo::machineUniqueId();
+ if (hostid.isEmpty())
+ QSKIP("Could not get a unique host ID on this machine");
+
+ QString lockFile = dir.path() + "/hostnameChangeLock";
+ QLockFile lock1(lockFile);
+ QVERIFY(lock1.lock());
+
+ {
+ // now modify it
+ QFile f;
+ QVERIFY(f.open(QLockFilePrivate::getLockFileHandle(&lock1),
+ QIODevice::ReadWrite | QIODevice::Text,
+ QFile::DontCloseHandle));
+ QVERIFY(overwriteLineInLockFile(f, 3, "this is not a hostname"));
+ }
+
+ {
+ // we should fail to lock
+ QLockFile lock2(lockFile);
+ QVERIFY(!lock2.tryLock(1000));
+ }
+}
+
+void tst_QLockFile::differentMachines()
+{
+ const QByteArray hostid = QSysInfo::machineUniqueId();
+ if (hostid.isEmpty())
+ QSKIP("Could not get a unique host ID on this machine");
+
+ QString lockFile = dir.path() + "/differentMachinesLock";
+ QLockFile lock1(lockFile);
+ QVERIFY(lock1.lock());
+
+ {
+ // now modify it
+ QFile f;
+ QVERIFY(f.open(QLockFilePrivate::getLockFileHandle(&lock1),
+ QIODevice::ReadWrite | QIODevice::Text,
+ QFile::DontCloseHandle));
+ QVERIFY(overwriteLineInLockFile(f, 1, QT_STRINGIFY(INT_MAX)));
+ QVERIFY(overwriteLineInLockFile(f, 4, "this is not a UUID"));
+ }
+
+ {
+ // we should fail to lock
+ QLockFile lock2(lockFile);
+ QVERIFY(!lock2.tryLock(1000));
+ }
+}
+
+void tst_QLockFile::reboot()
+{
+ const QByteArray bootid = QSysInfo::bootUniqueId();
+ if (bootid.isEmpty())
+ QSKIP("Could not get a unique boot ID on this machine");
+
+ // create a lock so we can get its contents
+ QString lockFile = dir.path() + "/rebootLock";
+ QLockFile lock1(lockFile);
+ QVERIFY(lock1.lock());
+
+ QFile f(lockFile);
+ QVERIFY(f.open(QFile::ReadOnly | QFile::Text));
+ auto lines = f.readAll().split('\n');
+ f.close();
+
+ lock1.unlock();
+
+ // now recreate the file simulating a reboot
+ QVERIFY(f.open(QFile::WriteOnly | QFile::Text));
+ lines[4] = "this is not a UUID";
+ f.write(lines.join('\n'));
+ f.close();
+
+ // we should succeed in locking
+ QVERIFY(lock1.tryLock(0));
+}
+
bool tst_QLockFile::overwritePidInLockFile(const QString &filePath, qint64 pid)
{
QFile f(filePath);
- if (!f.open(QFile::ReadWrite)) {
- qWarning("Cannot open %s.", qPrintable(filePath));
+ if (!f.open(QFile::ReadWrite | QFile::Text)) {
+ qErrnoWarning("Cannot open %s", qPrintable(filePath));
return false;
}
+ return overwriteLineInLockFile(f, 1, QString::number(pid));
+}
+
+bool tst_QLockFile::overwriteLineInLockFile(QFile &f, int line, const QString &newLine)
+{
+ f.seek(0);
QByteArray buf = f.readAll();
- int i = buf.indexOf('\n');
- if (i < 0) {
+ QStringList lines = QString::fromUtf8(buf).split('\n');
+ if (lines.size() < 3 && lines.size() < line - 1) {
qWarning("Unexpected lockfile content.");
return false;
}
- buf.remove(0, i);
- buf.prepend(QByteArray::number(pid));
+ lines[line - 1] = newLine;
f.seek(0);
+ buf = lines.join('\n').toUtf8();
f.resize(buf.size());
return f.write(buf) == buf.size();
}
diff --git a/tests/auto/corelib/io/qlockfile/tst_qlockfile.pro b/tests/auto/corelib/io/qlockfile/tst_qlockfile.pro
index 7a304fe779..da2660fd02 100644
--- a/tests/auto/corelib/io/qlockfile/tst_qlockfile.pro
+++ b/tests/auto/corelib/io/qlockfile/tst_qlockfile.pro
@@ -2,5 +2,5 @@ CONFIG += testcase
TARGET = tst_qlockfile
SOURCES += tst_qlockfile.cpp
-QT = core testlib concurrent
+QT = core-private testlib concurrent
win32:!winrt:LIBS += -ladvapi32