From 0aedca2f9756fa33420fb6b3f003f364424f9134 Mon Sep 17 00:00:00 2001
From: Thiago Macieira <thiago.macieira@intel.com>
Date: Thu, 30 Jul 2015 22:05:57 +0000
Subject: Revert "Fix performance of recursive read-write locks"

This reverts commit 666486b3efec871301b82244ec661e1eaa6cca9c, which
removed the QHash that protected against reader recursion
deadlocks. Without this hash, a reader will block on
d->readerWait.wait() until the writer finishes its task. However, the
writer never starts because there's a reader that hasn't released the
lock. That's a deadlock.

Change-Id: I792373bb361db35eb9e5504229c099008821a665
Task-number: QTBUG-47530
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
---
 src/corelib/thread/qreadwritelock.cpp | 57 +++++++++++++++++++++++++++++++++++
 src/corelib/thread/qreadwritelock_p.h |  1 +
 2 files changed, 58 insertions(+)

diff --git a/src/corelib/thread/qreadwritelock.cpp b/src/corelib/thread/qreadwritelock.cpp
index e6477bf5e0..0ea8c4c00f 100644
--- a/src/corelib/thread/qreadwritelock.cpp
+++ b/src/corelib/thread/qreadwritelock.cpp
@@ -136,11 +136,27 @@ void QReadWriteLock::lockForRead()
 {
     QMutexLocker lock(&d->mutex);
 
+    Qt::HANDLE self = 0;
+    if (d->recursive) {
+        self = QThread::currentThreadId();
+
+        QHash<Qt::HANDLE, int>::iterator it = d->currentReaders.find(self);
+        if (it != d->currentReaders.end()) {
+            ++it.value();
+            ++d->accessCount;
+            Q_ASSERT_X(d->accessCount > 0, "QReadWriteLock::lockForRead()",
+                       "Overflow in lock counter");
+            return;
+        }
+    }
+
     while (d->accessCount < 0 || d->waitingWriters) {
         ++d->waitingReaders;
         d->readerWait.wait(&d->mutex);
         --d->waitingReaders;
     }
+    if (d->recursive)
+        d->currentReaders.insert(self, 1);
 
     ++d->accessCount;
     Q_ASSERT_X(d->accessCount > 0, "QReadWriteLock::lockForRead()", "Overflow in lock counter");
@@ -166,8 +182,24 @@ bool QReadWriteLock::tryLockForRead()
 {
     QMutexLocker lock(&d->mutex);
 
+    Qt::HANDLE self = 0;
+    if (d->recursive) {
+        self = QThread::currentThreadId();
+
+        QHash<Qt::HANDLE, int>::iterator it = d->currentReaders.find(self);
+        if (it != d->currentReaders.end()) {
+            ++it.value();
+            ++d->accessCount;
+            Q_ASSERT_X(d->accessCount > 0, "QReadWriteLock::tryLockForRead()",
+                       "Overflow in lock counter");
+            return true;
+        }
+    }
+
     if (d->accessCount < 0)
         return false;
+    if (d->recursive)
+        d->currentReaders.insert(self, 1);
 
     ++d->accessCount;
     Q_ASSERT_X(d->accessCount > 0, "QReadWriteLock::tryLockForRead()", "Overflow in lock counter");
@@ -198,6 +230,20 @@ bool QReadWriteLock::tryLockForRead(int timeout)
 {
     QMutexLocker lock(&d->mutex);
 
+    Qt::HANDLE self = 0;
+    if (d->recursive) {
+        self = QThread::currentThreadId();
+
+        QHash<Qt::HANDLE, int>::iterator it = d->currentReaders.find(self);
+        if (it != d->currentReaders.end()) {
+            ++it.value();
+            ++d->accessCount;
+            Q_ASSERT_X(d->accessCount > 0, "QReadWriteLock::tryLockForRead()",
+                       "Overflow in lock counter");
+            return true;
+        }
+    }
+
     while (d->accessCount < 0 || d->waitingWriters) {
         ++d->waitingReaders;
         bool success = d->readerWait.wait(&d->mutex, timeout < 0 ? ULONG_MAX : ulong(timeout));
@@ -205,6 +251,8 @@ bool QReadWriteLock::tryLockForRead(int timeout)
         if (!success)
             return false;
     }
+    if (d->recursive)
+        d->currentReaders.insert(self, 1);
 
     ++d->accessCount;
     Q_ASSERT_X(d->accessCount > 0, "QReadWriteLock::tryLockForRead()", "Overflow in lock counter");
@@ -364,6 +412,15 @@ void QReadWriteLock::unlock()
     bool unlocked = false;
     if (d->accessCount > 0) {
         // releasing a read lock
+        if (d->recursive) {
+            Qt::HANDLE self = QThread::currentThreadId();
+            QHash<Qt::HANDLE, int>::iterator it = d->currentReaders.find(self);
+            if (it != d->currentReaders.end()) {
+                if (--it.value() <= 0)
+                    d->currentReaders.erase(it);
+            }
+        }
+
         unlocked = --d->accessCount == 0;
     } else if (d->accessCount < 0 && ++d->accessCount == 0) {
         // released a write lock
diff --git a/src/corelib/thread/qreadwritelock_p.h b/src/corelib/thread/qreadwritelock_p.h
index 15a6d1f57e..e57c0e403f 100644
--- a/src/corelib/thread/qreadwritelock_p.h
+++ b/src/corelib/thread/qreadwritelock_p.h
@@ -69,6 +69,7 @@ struct QReadWriteLockPrivate
 
     bool recursive;
     Qt::HANDLE currentWriter;
+    QHash<Qt::HANDLE, int> currentReaders;
 };
 
 QT_END_NAMESPACE
-- 
cgit v1.2.3