From eaceabe9ac0dbc659ba4683a0dd66effcec461f6 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 4 Jul 2019 16:59:32 +0200 Subject: QHostInfo: port from recursive to non-recursive mutex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It turns out that the only reason a recursive mutex is used was because work() was locking it. But work() was only ever called from functions that already had locked the mutex themselves, and kept it locked across the call to work(). Clearly mark work() as expecting to be called with the mutex held (by renaming the function to rescheduleWithMutexHeld(), then drop the mutex locking from it. After this change, a non-recursive mutex suffices, so save the memory allocation and extra complexity involved with recursive mutexes. Looking at the non-QT_CONFIG(thread) code in rescheduleWithMutexHeld(), one might be tempted to expect a recursive mutex, since that code calls QHostInfoRunnable::run() directly, which, in turn, recurses into QHostInfoLookupManager whence control came, under mutex lock. But in non-QT_CONFIG(thread) builds, QMutex is but an empty shell, all of its operations are no-ops, so no possibility for deadlock, either. Change-Id: Ic01d90c2ed3995b66ccf946d146fdaa6f9af3d8b Reviewed-by: Mårten Nordheim --- src/network/kernel/qhostinfo.cpp | 9 ++++----- src/network/kernel/qhostinfo_p.h | 6 ++++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/network/kernel/qhostinfo.cpp b/src/network/kernel/qhostinfo.cpp index a5e50ff21b..483ab875c6 100644 --- a/src/network/kernel/qhostinfo.cpp +++ b/src/network/kernel/qhostinfo.cpp @@ -944,10 +944,9 @@ void QHostInfoLookupManager::clear() cache.clear(); } -void QHostInfoLookupManager::work() +// assumes mutex is locked by caller +void QHostInfoLookupManager::rescheduleWithMutexHeld() { - QMutexLocker locker(&mutex); - if (wasDeleted) return; @@ -1012,7 +1011,7 @@ void QHostInfoLookupManager::scheduleLookup(QHostInfoRunnable *r) return; scheduledLookups.enqueue(r); - work(); + rescheduleWithMutexHeld(); } // called by QHostInfo @@ -1068,7 +1067,7 @@ void QHostInfoLookupManager::lookupFinished(QHostInfoRunnable *r) currentLookups.removeOne(r); #endif finishedLookups.append(r); - work(); + rescheduleWithMutexHeld(); } // This function returns immediately when we had a result in the cache, else it will later emit a signal diff --git a/src/network/kernel/qhostinfo_p.h b/src/network/kernel/qhostinfo_p.h index 0433d1d17b..f5b630346c 100644 --- a/src/network/kernel/qhostinfo_p.h +++ b/src/network/kernel/qhostinfo_p.h @@ -232,7 +232,6 @@ public: ~QHostInfoLookupManager(); void clear() override; - void work(); // called from QHostInfo void scheduleLookup(QHostInfoRunnable *r); @@ -255,10 +254,13 @@ protected: #if QT_CONFIG(thread) QThreadPool threadPool; #endif - QRecursiveMutex mutex; + QMutex mutex; bool wasDeleted; +private: + void rescheduleWithMutexHeld(); + private slots: #if QT_CONFIG(thread) void waitForThreadPoolDone() { threadPool.waitForDone(); } -- cgit v1.2.3