QgsCoordinateTransformContext must be thread safe

qgis · Dec 15, 2017 · 7af5545 · 7af5545
1 parent 5131258
commit 7af5545
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 15 deletions.
diff --git a/python/core/qgscoordinatetransformcontext.sip b/python/core/qgscoordinatetransformcontext.sip
@@ -27,6 +27,10 @@ class QgsCoordinateTransformContext
  addSourceDatumTransform() then this datum transform will be used. The same logic
  applies for destination CRS transforms set using addDestinationDatumTransform().
 
+.. note::
+
+   QgsCoordinateTransformContext objects are thread safe for read and write.
+
 .. note::
 
    QgsCoordinateTransformContext objects are implicitly shared.

diff --git a/src/core/qgscoordinatetransformcontext.cpp b/src/core/qgscoordinatetransformcontext.cpp
@@ -34,14 +34,21 @@ QgsCoordinateTransformContext &QgsCoordinateTransformContext::operator=( const Q
 void QgsCoordinateTransformContext::clear()
 {
   d.detach();
+  // play it safe
+  d->mLock.lockForWrite();
   d->mSourceDestDatumTransforms.clear();
   d->mSourceDatumTransforms.clear();
   d->mDestDatumTransforms.clear();
+  d->mLock.unlock();
 }
 
 QMap<QString, int> QgsCoordinateTransformContext::sourceDatumTransforms() const
 {
-  return d->mSourceDatumTransforms;
+  d->mLock.lockForRead();
+  auto res = d->mSourceDatumTransforms;
+  res.detach();
+  d->mLock.unlock();
+  return res;
 }
 
 bool QgsCoordinateTransformContext::addSourceDatumTransform( const QgsCoordinateReferenceSystem &crs, int transform )
@@ -50,19 +57,26 @@ bool QgsCoordinateTransformContext::addSourceDatumTransform( const QgsCoordinate
     return false;
 
   d.detach();
+  d->mLock.lockForWrite();
   if ( transform == -1 )
   {
     d->mSourceDatumTransforms.remove( crs.authid() );
-    return true;
   }
-
-  d->mSourceDatumTransforms.insert( crs.authid(), transform );
+  else
+  {
+    d->mSourceDatumTransforms.insert( crs.authid(), transform );
+  }
+  d->mLock.unlock();
   return true;
 }
 
 QMap<QString, int> QgsCoordinateTransformContext::destinationDatumTransforms() const
 {
-  return d->mDestDatumTransforms;
+  d->mLock.lockForRead();
+  auto res = d->mDestDatumTransforms;
+  res.detach();
+  d->mLock.unlock();
+  return res;
 }
 
 bool QgsCoordinateTransformContext::addDestinationDatumTransform( const QgsCoordinateReferenceSystem &crs, int transform )
@@ -71,19 +85,27 @@ bool QgsCoordinateTransformContext::addDestinationDatumTransform( const QgsCoord
     return false;
 
   d.detach();
+
+  d->mLock.lockForWrite();
   if ( transform == -1 )
   {
     d->mDestDatumTransforms.remove( crs.authid() );
-    return true;
   }
-
-  d->mDestDatumTransforms.insert( crs.authid(), transform );
+  else
+  {
+    d->mDestDatumTransforms.insert( crs.authid(), transform );
+  }
+  d->mLock.unlock();
   return true;
 }
 
 QMap<QPair<QString, QString>, QPair<int, int> > QgsCoordinateTransformContext::sourceDestinationDatumTransforms() const
 {
-  return d->mSourceDestDatumTransforms;
+  d->mLock.lockForRead();
+  auto res = d->mSourceDestDatumTransforms;
+  res.detach();
+  d->mLock.unlock();
+  return res;
 }
 
 bool QgsCoordinateTransformContext::addSourceDestinationDatumTransform( const QgsCoordinateReferenceSystem &sourceCrs, const QgsCoordinateReferenceSystem &destinationCrs, int sourceTransform, int destinationTransform )
@@ -92,13 +114,16 @@ bool QgsCoordinateTransformContext::addSourceDestinationDatumTransform( const Qg
     return false;
 
   d.detach();
+  d->mLock.lockForWrite();
   if ( sourceTransform == -1 || destinationTransform == -1 )
   {
     d->mSourceDestDatumTransforms.remove( qMakePair( sourceCrs.authid(), destinationCrs.authid() ) );
-    return true;
   }
-
-  d->mSourceDestDatumTransforms.insert( qMakePair( sourceCrs.authid(), destinationCrs.authid() ), qMakePair( sourceTransform, destinationTransform ) );
+  else
+  {
+    d->mSourceDestDatumTransforms.insert( qMakePair( sourceCrs.authid(), destinationCrs.authid() ), qMakePair( sourceTransform, destinationTransform ) );
+  }
+  d->mLock.unlock();
   return true;
 }
 

diff --git a/src/core/qgscoordinatetransformcontext.h b/src/core/qgscoordinatetransformcontext.h
@@ -44,6 +44,8 @@
  * addSourceDatumTransform() then this datum transform will be used. The same logic
  * applies for destination CRS transforms set using addDestinationDatumTransform().
  *
+ * \note QgsCoordinateTransformContext objects are thread safe for read and write.
+ *
  * \note QgsCoordinateTransformContext objects are implicitly shared.
  *
  * \since QGIS 3.0

diff --git a/src/core/qgscoordinatetransformcontext_p.h b/src/core/qgscoordinatetransformcontext_p.h
@@ -47,10 +47,12 @@ class QgsCoordinateTransformContextPrivate : public QSharedData
 
     QgsCoordinateTransformContextPrivate( const QgsCoordinateTransformContextPrivate &other )
       : QSharedData( other )
-      , mSourceDestDatumTransforms( other.mSourceDestDatumTransforms )
-      , mSourceDatumTransforms( other.mSourceDatumTransforms )
-      , mDestDatumTransforms( other.mDestDatumTransforms )
     {
+      other.mLock.lockForRead();
+      mSourceDestDatumTransforms = other.mSourceDestDatumTransforms;
+      mSourceDatumTransforms = other.mSourceDatumTransforms;
+      mDestDatumTransforms = other.mDestDatumTransforms;
+      other.mLock.unlock();
     }
 
     /**
@@ -65,6 +67,9 @@ class QgsCoordinateTransformContextPrivate : public QSharedData
     //! Mapping for datum transforms to use for destination CRS
     QMap< QString, int > mDestDatumTransforms;
 
+    //! Mutex for making QgsCoordinateTransformContextPrivate thread safe
+    mutable QReadWriteLock mLock;
+
 };