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>
This commit is contained in:
Thiago Macieira 2015-07-30 22:05:57 +00:00
parent 590d305f99
commit 0aedca2f97
2 changed files with 58 additions and 0 deletions

View File

@ -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

View File

@ -69,6 +69,7 @@ struct QReadWriteLockPrivate
bool recursive;
Qt::HANDLE currentWriter;
QHash<Qt::HANDLE, int> currentReaders;
};
QT_END_NAMESPACE