Do not access join layer from QgsVectorLayerFeatureIterator

This is not thread safe at all - we cannot access a layer from an iterator, as iterators may be running on background threads. Instead use a thread safe approach of storing a QgsVectorLayerFeatureSource and using that instead Fixes #38551 (cherry picked from commit b4d1dd8)
qgis · Jun 3, 2021 · cdbf722 · cdbf722
1 parent f041030
commit cdbf722
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 6 deletions.
diff --git a/python/core/auto_generated/qgsvectorlayerfeatureiterator.sip.in b/python/core/auto_generated/qgsvectorlayerfeatureiterator.sip.in
@@ -104,12 +104,18 @@ end of iterating: free the resources / lock
 %End
 
 
+
     struct FetchJoinInfo
     {
       const QgsVectorLayerJoinInfo *joinInfo;//!< Canonical source of information about the join
       QgsAttributeList attributes;      //!< Attributes to fetch
       int indexOffset;                  //!< At what position the joined fields start
-      QgsVectorLayer *joinLayer;        //!< Resolved pointer to the joined layer
+
+ QgsVectorLayer *joinLayer /Deprecated/;
+
+
+      QgsFields joinLayerFields;
+
       int targetField;                  //!< Index of field (of this layer) that drives the join
       int joinField;                    //!< Index of field (of the joined layer) must have equal value
 

diff --git a/python/core/auto_generated/qgsvectorlayerjoinbuffer.sip.in b/python/core/auto_generated/qgsvectorlayerjoinbuffer.sip.in
@@ -102,6 +102,13 @@ Find out what is the first index of the join within fields. Returns -1 if join i
 Returns a vector of indices for use in join based on field names from the layer
 
 .. versionadded:: 2.6
+%End
+
+    static QVector<int> joinSubsetIndices( const QgsFields &joinLayerFields, const QStringList &joinFieldsSubset );
+%Docstring
+Returns a vector of indices for use in join based on field names from the join layer's fields.
+
+.. versionadded:: 3.20
 %End
 
     QList<const QgsVectorLayerJoinInfo *> joinsWhereFieldIsId( const QgsField &field ) const;

diff --git a/src/core/qgsvectorlayerfeatureiterator.cpp b/src/core/qgsvectorlayerfeatureiterator.cpp
@@ -703,10 +703,14 @@ void QgsVectorLayerFeatureIterator::prepareJoin( int fieldIdx )
   {
     FetchJoinInfo info;
     info.joinInfo = joinInfo;
+    Q_NOWARN_DEPRECATED_PUSH
     info.joinLayer = joinLayer;
+    Q_NOWARN_DEPRECATED_POP
+    info.joinSource = std::make_shared< QgsVectorLayerFeatureSource >( joinLayer );
     info.indexOffset = mSource->mJoinBuffer->joinedFieldsOffset( joinInfo, mSource->mFields );
     info.targetField = mSource->mFields.indexFromName( joinInfo->targetFieldName() );
     info.joinField = joinLayer->fields().indexFromName( joinInfo->joinFieldName() );
+    info.joinLayerFields = joinLayer->fields();
 
     // for joined fields, we always need to request the targetField from the provider too
     if ( !mPreparedFields.contains( info.targetField ) && !mFieldsToPrepare.contains( info.targetField ) )
@@ -1043,11 +1047,13 @@ void QgsVectorLayerFeatureIterator::FetchJoinInfo::addJoinedAttributesCached( Qg
 
 void QgsVectorLayerFeatureIterator::FetchJoinInfo::addJoinedAttributesDirect( QgsFeature &f, const QVariant &joinValue ) const
 {
+#if 0 // this is not thread safe -- we cannot access the layer here as this will be called from non-main threads.
   // Shortcut
   if ( joinLayer && ! joinLayer->hasFeatures() )
   {
     return;
   }
+#endif
 
   // no memory cache, query the joined values by setting substring
   QString subsetString;
@@ -1085,7 +1091,7 @@ void QgsVectorLayerFeatureIterator::FetchJoinInfo::addJoinedAttributesDirect( Qg
   if ( joinInfo->hasSubset() )
   {
     const QStringList subsetNames = QgsVectorLayerJoinInfo::joinFieldNamesSubset( *joinInfo );
-    subsetIndices = QgsVectorLayerJoinBuffer::joinSubsetIndices( joinLayer, subsetNames );
+    subsetIndices = QgsVectorLayerJoinBuffer::joinSubsetIndices( joinLayerFields, subsetNames );
   }
 
   // select (no geometry)
@@ -1094,7 +1100,7 @@ void QgsVectorLayerFeatureIterator::FetchJoinInfo::addJoinedAttributesDirect( Qg
   request.setSubsetOfAttributes( attributes );
   request.setFilterExpression( subsetString );
   request.setLimit( 1 );
-  QgsFeatureIterator fi = joinLayer->getFeatures( request );
+  QgsFeatureIterator fi = joinSource->getFeatures( request );
 
   // get first feature
   QgsFeature fet;

diff --git a/src/core/qgsvectorlayerfeatureiterator.h b/src/core/qgsvectorlayerfeatureiterator.h
@@ -140,6 +140,8 @@ class CORE_EXPORT QgsVectorLayerFeatureIterator : public QgsAbstractFeatureItera
 
     void setInterruptionChecker( QgsFeedback *interruptionChecker ) override SIP_SKIP;
 
+    Q_NOWARN_DEPRECATED_PUSH
+
     /**
      * Join information prepared for fast attribute id mapping in QgsVectorLayerJoinBuffer::updateFeatureAttributes().
      * Created in the select() method of QgsVectorLayerJoinBuffer for the joins that contain fetched attributes
@@ -149,13 +151,32 @@ class CORE_EXPORT QgsVectorLayerFeatureIterator : public QgsAbstractFeatureItera
       const QgsVectorLayerJoinInfo *joinInfo;//!< Canonical source of information about the join
       QgsAttributeList attributes;      //!< Attributes to fetch
       int indexOffset;                  //!< At what position the joined fields start
-      QgsVectorLayer *joinLayer;        //!< Resolved pointer to the joined layer
+
+      /**
+       * Resolved pointer to the joined layer
+       * \deprecated Do NOT use. This is not thread safe, and should never be accessed from a background thread.
+       */
+      Q_DECL_DEPRECATED QgsVectorLayer *joinLayer SIP_DEPRECATED = nullptr;
+
+      /**
+       * Feature source for join
+       */
+      std::shared_ptr< QgsVectorLayerFeatureSource > joinSource SIP_SKIP;
+
+      /**
+       * Fields from joined layer.
+       *
+       * \since QGIS 3.20
+       */
+      QgsFields joinLayerFields;
+
       int targetField;                  //!< Index of field (of this layer) that drives the join
       int joinField;                    //!< Index of field (of the joined layer) must have equal value
 
       void addJoinedAttributesCached( QgsFeature &f, const QVariant &joinValue ) const;
       void addJoinedAttributesDirect( QgsFeature &f, const QVariant &joinValue ) const;
     };
+    Q_NOWARN_DEPRECATED_POP
 
 
     bool isValid() const override;

diff --git a/src/core/qgsvectorlayerjoinbuffer.cpp b/src/core/qgsvectorlayerjoinbuffer.cpp
@@ -181,13 +181,17 @@ void QgsVectorLayerJoinBuffer::cacheJoinLayer( QgsVectorLayerJoinInfo &joinInfo
 
 
 QVector<int> QgsVectorLayerJoinBuffer::joinSubsetIndices( QgsVectorLayer *joinLayer, const QStringList &joinFieldsSubset )
+{
+  return joinSubsetIndices( joinLayer->fields(), joinFieldsSubset );
+}
+
+QVector<int> QgsVectorLayerJoinBuffer::joinSubsetIndices( const QgsFields &joinLayerFields, const QStringList &joinFieldsSubset )
 {
   QVector<int> subsetIndices;
-  const QgsFields &fields = joinLayer->fields();
   for ( int i = 0; i < joinFieldsSubset.count(); ++i )
   {
     QString joinedFieldName = joinFieldsSubset.at( i );
-    int index = fields.lookupField( joinedFieldName );
+    int index = joinLayerFields.lookupField( joinedFieldName );
     if ( index != -1 )
     {
       subsetIndices.append( index );

diff --git a/src/core/qgsvectorlayerjoinbuffer.h b/src/core/qgsvectorlayerjoinbuffer.h
@@ -103,6 +103,12 @@ class CORE_EXPORT QgsVectorLayerJoinBuffer : public QObject, public QgsFeatureSi
      */
     static QVector<int> joinSubsetIndices( QgsVectorLayer *joinLayer, const QStringList &joinFieldsSubset );
 
+    /**
+     * Returns a vector of indices for use in join based on field names from the join layer's fields.
+     * \since QGIS 3.20
+     */
+    static QVector<int> joinSubsetIndices( const QgsFields &joinLayerFields, const QStringList &joinFieldsSubset );
+
     /**
      * Returns joins where the field of a target layer is considered as an id.
      * \param field the field of a target layer