From 7b9f7f3891230ddd2b2ef5cf3c760a605a02ee96 Mon Sep 17 00:00:00 2001
From: David Faure <david.faure@kdab.com>
Date: Thu, 4 Sep 2014 16:29:25 +0200
Subject: QLockFile: on Windows, retry deleting the lock file if it is being
 read.

A "sharing violation" happens when trying to delete the lock file and another
thread/process is reading it at that very moment. Detect the error and try again,
up to 10000 times - to avoid an infinite loop if QFile::remove fails for another
reason such as a sudden change of permissions preventing us from deleting our
own lock file.

On Unix the deletion can't fail because of readers, but it doesn't hurt
to check the return value there too, to catch other reasons for failures
such as a sudden permission change.

Task-number: QTBUG-38853
Change-Id: Icf12a74faed4a4916e3427abc09d9c33aa141476
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 src/corelib/io/qlockfile_unix.cpp                 |  5 ++++-
 src/corelib/io/qlockfile_win.cpp                  | 12 +++++++++++-
 tests/auto/corelib/io/qlockfile/tst_qlockfile.cpp | 20 ++++++++++++++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/corelib/io/qlockfile_unix.cpp b/src/corelib/io/qlockfile_unix.cpp
index d1ef9c1770..cd4cf3a50d 100644
--- a/src/corelib/io/qlockfile_unix.cpp
+++ b/src/corelib/io/qlockfile_unix.cpp
@@ -206,7 +206,10 @@ void QLockFile::unlock()
         return;
     close(d->fileHandle);
     d->fileHandle = -1;
-    QFile::remove(d->fileName);
+    if (!QFile::remove(d->fileName)) {
+        qWarning() << "Could not remove our own lock file" << d->fileName << "maybe permissions changed meanwhile?";
+        // This is bad because other users of this lock file will now have to wait for the stale-lock-timeout...
+    }
     d->lockError = QLockFile::NoError;
     d->isLocked = false;
 }
diff --git a/src/corelib/io/qlockfile_win.cpp b/src/corelib/io/qlockfile_win.cpp
index 00c8a85bcc..5f82270706 100644
--- a/src/corelib/io/qlockfile_win.cpp
+++ b/src/corelib/io/qlockfile_win.cpp
@@ -47,6 +47,7 @@
 #include "QtCore/qfileinfo.h"
 #include "QtCore/qdatetime.h"
 #include "QtCore/qdebug.h"
+#include "QtCore/qthread.h"
 
 QT_BEGIN_NAMESPACE
 
@@ -149,7 +150,16 @@ void QLockFile::unlock()
      if (!d->isLocked)
         return;
      CloseHandle(d->fileHandle);
-     QFile::remove(d->fileName);
+     int attempts = 0;
+     static const int maxAttempts = 500; // 500ms
+     while (!QFile::remove(d->fileName) && ++attempts < maxAttempts) {
+         // Someone is reading the lock file right now (on Windows this prevents deleting it).
+         QThread::msleep(1);
+     }
+     if (attempts == maxAttempts) {
+        qWarning() << "Could not remove our own lock file" << d->fileName << ". Either other users of the lock file are reading it constantly for 500 ms, or we (no longer) have permissions to delete the file";
+        // This is bad because other users of this lock file will now have to wait for the stale-lock-timeout...
+     }
      d->lockError = QLockFile::NoError;
      d->isLocked = false;
 }
diff --git a/tests/auto/corelib/io/qlockfile/tst_qlockfile.cpp b/tests/auto/corelib/io/qlockfile/tst_qlockfile.cpp
index bd9e28beb5..de5d641c57 100644
--- a/tests/auto/corelib/io/qlockfile/tst_qlockfile.cpp
+++ b/tests/auto/corelib/io/qlockfile/tst_qlockfile.cpp
@@ -57,6 +57,7 @@ private slots:
     void lockUnlock();
     void lockOutOtherProcess();
     void lockOutOtherThread();
+    void raceWithOtherThread();
     void waitForLock_data();
     void waitForLock();
     void staleLockFromCrashedProcess_data();
@@ -165,6 +166,25 @@ void tst_QLockFile::lockOutOtherThread()
     QCOMPARE(ret2.result(), QLockFile::NoError);
 }
 
+static QLockFile::LockError lockFromThread(const QString &fileName)
+{
+    QLockFile lockInThread(fileName);
+    lockInThread.lock();
+    return lockInThread.error();
+}
+
+// QTBUG-38853, best way to trigger it was to add a QThread::sleep(1) in QLockFilePrivate::getLockInfo() after the first readLine.
+// Then (on Windows), the QFile::remove() in unlock() (called by the first thread who got the lock, in the destructor)
+// would fail due to the existing reader on the file. Fixed by checking the return value of QFile::remove() in unlock().
+void tst_QLockFile::raceWithOtherThread()
+{
+    const QString fileName = dir.path() + "/raceWithOtherThread";
+    QFuture<QLockFile::LockError> ret = QtConcurrent::run<QLockFile::LockError>(lockFromThread, fileName);
+    QFuture<QLockFile::LockError> ret2 = QtConcurrent::run<QLockFile::LockError>(lockFromThread, fileName);
+    QCOMPARE(ret.result(), QLockFile::NoError);
+    QCOMPARE(ret2.result(), QLockFile::NoError);
+}
+
 static bool lockFromThread(const QString &fileName, int sleepMs, QSemaphore *semThreadReady, QSemaphore *semMainThreadDone)
 {
     QLockFile lockFile(fileName);
-- 
cgit v1.2.3