RAII for QgsCoordinateTransform locking

I experienced a freeze where the main thread was stuck in QgsCoordinateTransformPrivate::freeProj(), waiting for a write lock. None of the other threads had anything useful in their stack trace that would have explained why the lock was not available. The only explanation I can come up with is that an exception or thread termination was messing with the lock, leaving it in a locked state. By using RAII we can avoid this scenario.
qgis · Dec 17, 2018 · 1730a40 · 1730a40
1 parent bebc34b
commit 1730a40
Showing 1 changed file with 4 additions and 7 deletions.
diff --git a/src/core/qgscoordinatetransform_p.cpp b/src/core/qgscoordinatetransform_p.cpp
@@ -18,6 +18,7 @@
 #include "qgscoordinatetransform_p.h"
 #include "qgslogger.h"
 #include "qgsapplication.h"
+#include "qgsreadwritelocker.h"
 
 extern "C"
 {
@@ -225,7 +226,7 @@ void QgsCoordinateTransformPrivate::calculateTransforms( const QgsCoordinateTran
 
 QPair<projPJ, projPJ> QgsCoordinateTransformPrivate::threadLocalProjData()
 {
-  mProjLock.lockForRead();
+  QgsReadWriteLocker locker( mProjLock, QgsReadWriteLocker::Read );
 
 #ifdef USE_THREAD_LOCAL
   QMap < uintptr_t, QPair< projPJ, projPJ > >::const_iterator it = mProjProjections.constFind( reinterpret_cast< uintptr_t>( mProjContext.get() ) );
@@ -246,13 +247,11 @@ QPair<projPJ, projPJ> QgsCoordinateTransformPrivate::threadLocalProjData()
   if ( it != mProjProjections.constEnd() )
   {
     QPair<projPJ, projPJ> res = it.value();
-    mProjLock.unlock();
     return res;
   }
 
   // proj projections don't exist yet, so we need to create
-  mProjLock.unlock();
-  mProjLock.lockForWrite();
+  locker.changeMode( QgsReadWriteLocker::Write );
 
 #ifdef USE_THREAD_LOCAL
   QPair<projPJ, projPJ> res = qMakePair( pj_init_plus_ctx( mProjContext.get(), mSourceProjString.toUtf8() ),
@@ -263,7 +262,6 @@ QPair<projPJ, projPJ> QgsCoordinateTransformPrivate::threadLocalProjData()
                                          pj_init_plus_ctx( pContext, mDestProjString.toUtf8() ) );
   mProjProjections.insert( reinterpret_cast< uintptr_t>( pContext ), res );
 #endif
-  mProjLock.unlock();
   return res;
 }
 
@@ -332,15 +330,14 @@ void QgsCoordinateTransformPrivate::setFinder()
 
 void QgsCoordinateTransformPrivate::freeProj()
 {
-  mProjLock.lockForWrite();
+  QgsReadWriteLocker locker( mProjLock, QgsReadWriteLocker::Write );
   QMap < uintptr_t, QPair< projPJ, projPJ > >::const_iterator it = mProjProjections.constBegin();
   for ( ; it != mProjProjections.constEnd(); ++it )
   {
     pj_free( it.value().first );
     pj_free( it.value().second );
   }
   mProjProjections.clear();
-  mProjLock.unlock();
 }
 
 ///@endcond