Never assume anything

m-kuhn · m-kuhn · commit 44d7644ef952 · 2019-03-02T23:25:40.000Z
because when you do, reality will teach you better
diff --git a/src/analysis/vector/geometry_checker/qgsfeaturepool.cpp b/src/analysis/vector/geometry_checker/qgsfeaturepool.cpp
@@ -38,7 +38,15 @@ QgsFeaturePool::QgsFeaturePool( QgsVectorLayer *layer )
 
 bool QgsFeaturePool::getFeature( QgsFeatureId id, QgsFeature &feature, QgsFeedback *feedback )
 {
-  QgsReadWriteLocker locker( mCacheLock, QgsReadWriteLocker::Read );
+  // Why is there a write lock acquired here? Weird, we only want to read a feature from the cache, right?
+  // A method like `QCache::object(const Key &key) const` certainly would not modify its internals.
+  // Mmmh. What if reality was different?
+  // If one reads the docs very, very carefully one will find the term "reentrant" in the
+  // small print of the QCache docs. This is the hint that reality is different.
+  //
+  // https://bugreports.qt.io/browse/QTBUG-19794
+
+  QgsReadWriteLocker locker( mCacheLock, QgsReadWriteLocker::Write );
   QgsFeature *cachedFeature = mFeatureCache.object( id );
   if ( cachedFeature )
   {
