QLockInfo: Centralize the management of the lock file's contents

We duplicated both the creation of the contents and the check if the
file was stale. Centralize everything in qlockfile.cpp.

Change-Id: I0b48fc8e90304e0dacc3fffd14e91174af79841f
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@qt.io>
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>

4 files changed, 100 insertions, 77 deletions

diff --git a/src/corelib/io/qlockfile.cpp b/src/corelib/io/qlockfile.cpp
index 48317d07e0..197b1f80e5 100644
--- a/src/corelib/io/qlockfile.cpp
+++ b/src/corelib/io/qlockfile.cpp
@@ -2,6 +2,7 @@
 **
 ** Copyright (C) 2013 David Faure <faure+bluesystems@kde.org>
 ** Copyright (C) 2016 The Qt Company Ltd.
+** Copyright (C) 2017 Intel Corporation.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the QtCore module of the Qt Toolkit.
@@ -42,12 +43,34 @@
 #include "qlockfile_p.h"
 
 #include <QtCore/qthread.h>
+#include <QtCore/qcoreapplication.h>
 #include <QtCore/qdeadlinetimer.h>
 #include <QtCore/qdatetime.h>
 #include <QtCore/qfileinfo.h>
 
 QT_BEGIN_NAMESPACE
 
+namespace {
+struct LockFileInfo
+{
+    qint64 pid;
+    QString appname;
+    QString hostname;
+};
+}
+
+static bool getLockInfo_helper(const QString &fileName, LockFileInfo *info);
+
+static QString machineName()
+{
+#ifdef Q_OS_WIN
+    // we don't use QSysInfo because it tries to do name resolution
+    return qEnvironmentVariable("COMPUTERNAME");
+#else
+    return QSysInfo::machineHostName();
+#endif
+}
+
 /*!
     \class QLockFile
     \inmodule QtCore
@@ -291,10 +314,27 @@ bool QLockFile::tryLock(int timeout)
 bool QLockFile::getLockInfo(qint64 *pid, QString *hostname, QString *appname) const
 {
     Q_D(const QLockFile);
-    return d->getLockInfo(pid, hostname, appname);
+    LockFileInfo info;
+    if (!getLockInfo_helper(d->fileName, &info))
+        return false;
+    if (pid)
+        *pid = info.pid;
+    if (hostname)
+        *hostname = info.hostname;
+    if (appname)
+        *appname = info.appname;
+    return true;
 }
 
-bool QLockFilePrivate::getLockInfo(qint64 *pid, QString *hostname, QString *appname) const
+QByteArray QLockFilePrivate::lockFileContents() const
+{
+    // Use operator% from the fast builder to avoid multiple memory allocations.
+    return QByteArray::number(QCoreApplication::applicationPid()) % '\n'
+            % QCoreApplication::applicationName().toUtf8() % '\n'
+            % machineName().toUtf8() % '\n';
+}
+
+static bool getLockInfo_helper(const QString &fileName, LockFileInfo *info)
 {
     QFile reader(fileName);
     if (!reader.open(QIODevice::ReadOnly))
@@ -309,14 +349,25 @@ bool QLockFilePrivate::getLockInfo(qint64 *pid, QString *hostname, QString *appn
     QByteArray hostNameLine = reader.readLine();
     hostNameLine.chop(1);
 
-    qint64 thePid = pidLine.toLongLong();
-    if (pid)
-        *pid = thePid;
-    if (appname)
-        *appname = QString::fromUtf8(appNameLine);
-    if (hostname)
-        *hostname = QString::fromUtf8(hostNameLine);
-    return thePid > 0;
+    bool ok;
+    info->appname = QString::fromUtf8(appNameLine);
+    info->hostname = QString::fromUtf8(hostNameLine);
+    info->pid = pidLine.toLongLong(&ok);
+    return ok && info->pid > 0;
+}
+
+bool QLockFilePrivate::isApparentlyStale() const
+{
+    LockFileInfo info;
+    if (getLockInfo_helper(fileName, &info)) {
+        if (info.hostname.isEmpty() || info.hostname == machineName()) {
+            if (!isProcessRunning(info.pid, info.appname))
+                return true;
+        }
+    }
+
+    const qint64 age = QFileInfo(fileName).lastModified().msecsTo(QDateTime::currentDateTimeUtc());
+    return staleLockTime > 0 && qAbs(age) > staleLockTime;
 }
 
 /*!
diff --git a/src/corelib/io/qlockfile_p.h b/src/corelib/io/qlockfile_p.h
index a6f61b20f3..b41dfb38ad 100644
--- a/src/corelib/io/qlockfile_p.h
+++ b/src/corelib/io/qlockfile_p.h
@@ -78,12 +78,14 @@ public:
     }
     QLockFile::LockError tryLock_sys();
     bool removeStaleLock();
-    bool getLockInfo(qint64 *pid, QString *hostname, QString *appname) const;
+    QByteArray lockFileContents() const;
     // 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;
+
     // used in dbusmenu
     Q_CORE_EXPORT static QString processNameByPid(qint64 pid);
+    static bool isProcessRunning(qint64 pid, const QString &appname);
 
     QString fileName;
 #ifdef Q_OS_WIN
diff --git a/src/corelib/io/qlockfile_unix.cpp b/src/corelib/io/qlockfile_unix.cpp
index 30531d910e..fc01f83e80 100644
--- a/src/corelib/io/qlockfile_unix.cpp
+++ b/src/corelib/io/qlockfile_unix.cpp
@@ -1,7 +1,7 @@
 /****************************************************************************
 **
 ** Copyright (C) 2013 David Faure <faure+bluesystems@kde.org>
-** Copyright (C) 2016 Intel Corporation.
+** Copyright (C) 2017 Intel Corporation.
 ** Copyright (C) 2016 The Qt Company Ltd.
 ** Contact: https://www.qt.io/licensing/
 **
@@ -42,7 +42,6 @@
 #include "private/qlockfile_p.h"
 
 #include "QtCore/qtemporaryfile.h"
-#include "QtCore/qcoreapplication.h"
 #include "QtCore/qfileinfo.h"
 #include "QtCore/qdebug.h"
 #include "QtCore/qdatetime.h"
@@ -147,13 +146,6 @@ static bool setNativeLocks(int fd)
 
 QLockFile::LockError QLockFilePrivate::tryLock_sys()
 {
-    // Assemble data, to write in a single call to write
-    // (otherwise we'd have to check every write call)
-    // Use operator% from the fast builder to avoid multiple memory allocations.
-    QByteArray fileData = QByteArray::number(QCoreApplication::applicationPid()) % '\n'
-                          % QCoreApplication::applicationName().toUtf8() % '\n'
-                          % QSysInfo::machineHostName().toUtf8() % '\n';
-
     const QByteArray lockFileName = QFile::encodeName(fileName);
     const int fd = qt_safe_open(lockFileName.constData(), O_WRONLY | O_CREAT | O_EXCL, 0666);
     if (fd < 0) {
@@ -173,6 +165,7 @@ QLockFile::LockError QLockFilePrivate::tryLock_sys()
         qWarning() << "setNativeLocks failed:" << qt_error_string(errnoSaved);
     }
 
+    QByteArray fileData = lockFileContents();
     if (qt_write_loop(fd, fileData.constData(), fileData.size()) < fileData.size()) {
         qt_safe_close(fd);
         if (!QFile::remove(fileName))
@@ -204,26 +197,21 @@ bool QLockFilePrivate::removeStaleLock()
     return success;
 }
 
-bool QLockFilePrivate::isApparentlyStale() const
+bool QLockFilePrivate::isProcessRunning(qint64 pid, const QString &appname)
 {
-    qint64 pid;
-    QString hostname, appname;
-    if (getLockInfo(&pid, &hostname, &appname)) {
-        if (hostname.isEmpty() || hostname == QSysInfo::machineHostName()) {
-            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.
-            }
-        }
+    if (::kill(pid, 0) == -1 && errno == ESRCH)
+        return false; // 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 false;   // PID got reused by a different application.
     }
-    const qint64 age = QFileInfo(fileName).lastModified().msecsTo(QDateTime::currentDateTime());
-    return staleLockTime > 0 && qAbs(age) > staleLockTime;
+
+    return true;
 }
 
 QString QLockFilePrivate::processNameByPid(qint64 pid)
diff --git a/src/corelib/io/qlockfile_win.cpp b/src/corelib/io/qlockfile_win.cpp
index 4b43181686..de64ec0432 100644
--- a/src/corelib/io/qlockfile_win.cpp
+++ b/src/corelib/io/qlockfile_win.cpp
@@ -2,6 +2,7 @@
 **
 ** Copyright (C) 2013 David Faure <faure+bluesystems@kde.org>
 ** Copyright (C) 2016 The Qt Company Ltd.
+** Copyright (C) 2017 Intel Corporation.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the QtCore module of the Qt Toolkit.
@@ -42,7 +43,6 @@
 #include "private/qfilesystementry_p.h"
 #include <qt_windows.h>
 
-#include "QtCore/qcoreapplication.h"
 #include "QtCore/qfileinfo.h"
 #include "QtCore/qdatetime.h"
 #include "QtCore/qdebug.h"
@@ -50,11 +50,6 @@
 
 QT_BEGIN_NAMESPACE
 
-static inline QByteArray localHostName()
-{
-    return qgetenv("COMPUTERNAME");
-}
-
 static inline bool fileExists(const wchar_t *fileName)
 {
     WIN32_FILE_ATTRIBUTE_DATA  data;
@@ -107,15 +102,7 @@ QLockFile::LockError QLockFilePrivate::tryLock_sys()
 
     // We hold the lock, continue.
     fileHandle = fh;
-    // Assemble data, to write in a single call to write
-    // (otherwise we'd have to check every write call)
-    QByteArray fileData;
-    fileData += QByteArray::number(QCoreApplication::applicationPid());
-    fileData += '\n';
-    fileData += QCoreApplication::applicationName().toUtf8();
-    fileData += '\n';
-    fileData += localHostName();
-    fileData += '\n';
+    QByteArray fileData = lockFileContents();
     DWORD bytesWritten = 0;
     QLockFile::LockError error = QLockFile::NoError;
     if (!WriteFile(fh, fileData.constData(), fileData.size(), &bytesWritten, NULL) || !FlushFileBuffers(fh))
@@ -129,38 +116,33 @@ bool QLockFilePrivate::removeStaleLock()
     return QFile::remove(fileName);
 }
 
-bool QLockFilePrivate::isApparentlyStale() const
+bool QLockFilePrivate::isProcessRunning(qint64 pid, const QString &appname)
 {
-    qint64 pid;
-    QString hostname, appname;
-
     // On WinRT there seems to be no way of obtaining information about other
     // processes due to sandboxing
 #ifndef Q_OS_WINRT
-    if (getLockInfo(&pid, &hostname, &appname)) {
-        if (hostname.isEmpty() || hostname == QString::fromLocal8Bit(localHostName())) {
-            HANDLE procHandle = ::OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, pid);
-            if (!procHandle)
-                return true;
-            // We got a handle but check if process is still alive
-            DWORD exitCode = 0;
-            if (!::GetExitCodeProcess(procHandle, &exitCode))
-                exitCode = 0;
-            ::CloseHandle(procHandle);
-            if (exitCode != STILL_ACTIVE)
-                return true;
-            const QString processName = processNameByPid(pid);
-            if (!processName.isEmpty() && processName != appname)
-                return true; // PID got reused by a different application.
-        }
-    }
+    HANDLE procHandle = ::OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, pid);
+    if (!procHandle)
+        return false;
+
+    // We got a handle but check if process is still alive
+    DWORD exitCode = 0;
+    if (!::GetExitCodeProcess(procHandle, &exitCode))
+        exitCode = 0;
+    ::CloseHandle(procHandle);
+    if (exitCode != STILL_ACTIVE)
+        return false;
+
+    const QString processName = processNameByPid(pid);
+    if (!processName.isEmpty() && processName != appname)
+        return false; // PID got reused by a different application.
+
 #else // !Q_OS_WINRT
     Q_UNUSED(pid);
-    Q_UNUSED(hostname);
     Q_UNUSED(appname);
 #endif // Q_OS_WINRT
-    const qint64 age = QFileInfo(fileName).lastModified().msecsTo(QDateTime::currentDateTime());
-    return staleLockTime > 0 && qAbs(age) > staleLockTime;
+
+    return true;
 }
 
 QString QLockFilePrivate::processNameByPid(qint64 pid)