Merge pull request #4394 from nyalldawson/bad_iterator_go_and_sit_in_…

…a_corner Fix crash when OGR layer is removed when source is stored
qgis · Apr 23, 2017 · 716ff6c · 716ff6c
2 parents 82c66f8 + 94c56aa
commit 716ff6c
Show file tree

Hide file tree

Showing 30 changed files with 195 additions and 167 deletions.
diff --git a/python/core/qgsfeaturerequest.sip b/python/core/qgsfeaturerequest.sip
@@ -343,7 +343,7 @@ class QgsAbstractFeatureSource
      * @param request The request
      * @return A feature iterator
      */
-    virtual QgsFeatureIterator getFeatures( const QgsFeatureRequest& request ) = 0;
+    virtual QgsFeatureIterator getFeatures( const QgsFeatureRequest& request = QgsFeatureRequest() ) = 0;
 
   protected:
     void iteratorOpened( QgsAbstractFeatureIterator* it );

diff --git a/python/core/qgsvectorlayerfeatureiterator.sip b/python/core/qgsvectorlayerfeatureiterator.sip
@@ -2,6 +2,23 @@
 // abstract feature iterator implementations are not part of public API
 
 
+class QgsVectorLayerFeatureSource : QgsAbstractFeatureSource
+{
+%TypeHeaderCode
+#include <qgsvectorlayerfeatureiterator.h>
+%End
+  public:
+
+    /** Constructor for QgsVectorLayerFeatureSource.
+     * \param layer source layer
+     */
+    explicit QgsVectorLayerFeatureSource( const QgsVectorLayer *layer );
+
+    ~QgsVectorLayerFeatureSource();
+
+    virtual QgsFeatureIterator getFeatures( const QgsFeatureRequest &request = QgsFeatureRequest() );
+};
+
 class QgsVectorLayerFeatureIterator : QgsAbstractFeatureIterator
 {
 %TypeHeaderCode

diff --git a/src/core/qgsfeaturerequest.h b/src/core/qgsfeaturerequest.h
@@ -437,7 +437,7 @@ class CORE_EXPORT QgsAbstractFeatureSource
      * \param request The request
      * \returns A feature iterator
      */
-    virtual QgsFeatureIterator getFeatures( const QgsFeatureRequest &request ) = 0;
+    virtual QgsFeatureIterator getFeatures( const QgsFeatureRequest &request = QgsFeatureRequest() ) = 0;
 
   protected:
     void iteratorOpened( QgsAbstractFeatureIterator *it );

diff --git a/src/core/qgsvectorlayerfeatureiterator.h b/src/core/qgsvectorlayerfeatureiterator.h
@@ -36,7 +36,6 @@ class QgsVectorLayerFeatureIterator;
 
 /** \ingroup core
  * Partial snapshot of vector layer's state (only the members necessary for access to features)
- * \note not available in Python bindings
 */
 class CORE_EXPORT QgsVectorLayerFeatureSource : public QgsAbstractFeatureSource
 {
@@ -49,7 +48,7 @@ class CORE_EXPORT QgsVectorLayerFeatureSource : public QgsAbstractFeatureSource
 
     ~QgsVectorLayerFeatureSource();
 
-    virtual QgsFeatureIterator getFeatures( const QgsFeatureRequest &request ) override;
+    virtual QgsFeatureIterator getFeatures( const QgsFeatureRequest &request = QgsFeatureRequest() ) override;
 
     friend class QgsVectorLayerFeatureIterator;
 

diff --git a/src/providers/arcgisrest/CMakeLists.txt b/src/providers/arcgisrest/CMakeLists.txt
@@ -25,7 +25,6 @@ SET (AFS_SRCS
 SET (AFS_MOC_HDRS
   qgsarcgisrestutils.h
   qgsafsdataitems.h
-  qgsafsfeatureiterator.h
   qgsafsprovider.h
   qgsafssourceselect.h
 )

diff --git a/src/providers/arcgisrest/qgsafsfeatureiterator.cpp b/src/providers/arcgisrest/qgsafsfeatureiterator.cpp
@@ -39,9 +39,7 @@ QgsAfsProvider *QgsAfsFeatureSource::provider() const
 
 QgsAfsFeatureIterator::QgsAfsFeatureIterator( QgsAfsFeatureSource *source, bool ownSource, const QgsFeatureRequest &request )
   : QgsAbstractFeatureIteratorFromSource<QgsAfsFeatureSource>( source, ownSource, request )
-{
-  mFeatureIterator = 0;
-}
+{}
 
 QgsAfsFeatureIterator::~QgsAfsFeatureIterator()
 {

diff --git a/src/providers/arcgisrest/qgsafsfeatureiterator.h b/src/providers/arcgisrest/qgsafsfeatureiterator.h
@@ -21,18 +21,14 @@ class QgsAfsProvider;
 class QgsSpatialIndex;
 
 
-class QgsAfsFeatureSource : public QObject, public QgsAbstractFeatureSource
+class QgsAfsFeatureSource : public QgsAbstractFeatureSource
 {
-    Q_OBJECT
 
   public:
     QgsAfsFeatureSource( const QgsAfsProvider *provider );
     QgsFeatureIterator getFeatures( const QgsFeatureRequest &request ) override;
     QgsAfsProvider *provider() const;
 
-  signals:
-    void extentRequested( const QgsRectangle & );
-
   protected:
     QgsAfsProvider *mProvider = nullptr;
 
@@ -51,7 +47,7 @@ class QgsAfsFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsAfs
     bool fetchFeature( QgsFeature &f ) override;
 
   private:
-    QgsFeatureId mFeatureIterator;
+    QgsFeatureId mFeatureIterator = 0;
 };
 
 #endif // QGSAFSFEATUREITERATOR_H
diff --git a/src/providers/db2/qgsdb2featureiterator.cpp b/src/providers/db2/qgsdb2featureiterator.cpp
@@ -32,8 +32,6 @@ QgsDb2FeatureIterator::QgsDb2FeatureIterator( QgsDb2FeatureSource *source, bool
   : QgsAbstractFeatureIteratorFromSource<QgsDb2FeatureSource>( source, ownSource, request )
 {
   mClosed = false;
-  mQuery = nullptr;
-  mFetchCount = 0;
 
   BuildStatement( request );
 
@@ -48,7 +46,7 @@ QgsDb2FeatureIterator::QgsDb2FeatureIterator( QgsDb2FeatureSource *source, bool
   }
 
   // create sql query
-  mQuery = new QSqlQuery( mDatabase );
+  mQuery.reset( new QSqlQuery( mDatabase ) );
 
   // start selection
   rewind();
@@ -428,7 +426,7 @@ bool QgsDb2FeatureIterator::close()
     {
       mQuery->finish();
     }
-    delete mQuery;
+    mQuery.reset();
   }
 
   if ( mDatabase.isOpen() )
@@ -447,15 +445,14 @@ bool QgsDb2FeatureIterator::close()
 QgsDb2FeatureSource::QgsDb2FeatureSource( const QgsDb2Provider *p )
   : mFields( p->mAttributeFields )
   , mFidColName( p->mFidColName )
+  , mSRId( p->mSRId )
   , mGeometryColName( p->mGeometryColName )
   , mGeometryColType( p->mGeometryColType )
   , mSchemaName( p->mSchemaName )
   , mTableName( p->mTableName )
   , mConnInfo( p->mConnInfo )
   , mSqlWhereClause( p->mSqlWhereClause )
-{
-  mSRId = p->mSRId;
-}
+{}
 
 QgsDb2FeatureSource::~QgsDb2FeatureSource()
 {

diff --git a/src/providers/db2/qgsdb2featureiterator.h b/src/providers/db2/qgsdb2featureiterator.h
@@ -34,7 +34,7 @@ class QgsDb2FeatureSource : public QgsAbstractFeatureSource
 
     virtual QgsFeatureIterator getFeatures( const QgsFeatureRequest &request ) override;
 
-  protected:
+  private:
     QgsFields mFields;
     QString mFidColName;
     long mSRId;
@@ -85,7 +85,7 @@ class QgsDb2FeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsDb2
     QString mOrderByClause;
 
     // The current sql query
-    QSqlQuery *mQuery = nullptr;
+    std::unique_ptr< QSqlQuery > mQuery;
 
     // The current sql statement
     QString mStatement;
@@ -99,7 +99,7 @@ class QgsDb2FeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsDb2
     bool mExpressionCompiled;
     bool mOrderByCompiled;
 
-    int mFetchCount;
+    int mFetchCount = 0;
 };
 
 #endif // QGSDB2FEATUREITERATOR_H
diff --git a/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp b/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp
@@ -28,8 +28,7 @@
 
 QgsDelimitedTextFeatureIterator::QgsDelimitedTextFeatureIterator( QgsDelimitedTextFeatureSource *source, bool ownSource, const QgsFeatureRequest &request )
   : QgsAbstractFeatureIteratorFromSource<QgsDelimitedTextFeatureSource>( source, ownSource, request )
-  , mNextId( 0 )
-  , mTestGeometryExact( false )
+  , mTestSubset( mSource->mSubsetExpression )
 {
 
   // Determine mode to use based on request...
@@ -39,14 +38,6 @@ QgsDelimitedTextFeatureIterator::QgsDelimitedTextFeatureIterator( QgsDelimitedTe
   // load it.
   bool hasGeometry = mSource->mGeomRep != QgsDelimitedTextProvider::GeomNone;
 
-  // Does the layer have an explicit or implicit subset (implicit subset is if we have geometry which can
-  // be invalid)
-
-  mTestSubset = mSource->mSubsetExpression;
-  mTestGeometry = false;
-
-  mMode = FileScan;
-
   if ( !request.filterRect().isNull() && hasGeometry )
   {
     QgsDebugMsg( "Configuring for rectangle select" );
@@ -271,7 +262,7 @@ bool QgsDelimitedTextFeatureIterator::nextFeatureInternal( QgsFeature &feature )
 {
   QStringList tokens;
 
-  QgsDelimitedTextFile *file = mSource->mFile;
+  QgsDelimitedTextFile *file = mSource->mFile.get();
 
   // If the iterator is not scanning the file, then it will have requested a specific
   // record, so only need to load that one.
@@ -506,7 +497,7 @@ QgsDelimitedTextFeatureSource::QgsDelimitedTextFeatureSource( const QgsDelimited
     url.removeQueryItem( QStringLiteral( "watchFile" ) );
   }
 
-  mFile = new QgsDelimitedTextFile();
+  mFile.reset( new QgsDelimitedTextFile() );
   mFile->setFromUrl( url );
 
   mExpressionContext << QgsExpressionContextUtils::globalScope()
@@ -517,8 +508,6 @@ QgsDelimitedTextFeatureSource::QgsDelimitedTextFeatureSource( const QgsDelimited
 QgsDelimitedTextFeatureSource::~QgsDelimitedTextFeatureSource()
 {
   delete mSubsetExpression;
-  delete mSpatialIndex;
-  delete mFile;
 }
 
 QgsFeatureIterator QgsDelimitedTextFeatureSource::getFeatures( const QgsFeatureRequest &request )

diff --git a/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.h b/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.h
@@ -30,16 +30,16 @@ class QgsDelimitedTextFeatureSource : public QgsAbstractFeatureSource
 
     virtual QgsFeatureIterator getFeatures( const QgsFeatureRequest &request ) override;
 
-  protected:
+  private:
     QgsDelimitedTextProvider::GeomRepresentationType mGeomRep;
     QgsExpression *mSubsetExpression = nullptr;
     QgsExpressionContext mExpressionContext;
     QgsRectangle mExtent;
     bool mUseSpatialIndex;
-    QgsSpatialIndex *mSpatialIndex = nullptr;
+    std::unique_ptr< QgsSpatialIndex > mSpatialIndex;
     bool mUseSubsetIndex;
     QList<quintptr> mSubsetIndex;
-    QgsDelimitedTextFile *mFile = nullptr;
+    std::unique_ptr< QgsDelimitedTextFile > mFile;
     QgsFields mFields;
     int mFieldCount;  // Note: this includes field count for wkt field
     int mXFieldIndex;
@@ -78,6 +78,8 @@ class QgsDelimitedTextFeatureIterator : public QgsAbstractFeatureIteratorFromSou
   protected:
     virtual bool fetchFeature( QgsFeature &feature ) override;
 
+  private:
+
     bool setNextFeatureId( qint64 fid );
 
     bool nextFeatureInternal( QgsFeature &feature );
@@ -86,12 +88,12 @@ class QgsDelimitedTextFeatureIterator : public QgsAbstractFeatureIteratorFromSou
     void fetchAttribute( QgsFeature &feature, int fieldIdx, const QStringList &tokens );
 
     QList<QgsFeatureId> mFeatureIds;
-    IteratorMode mMode;
-    long mNextId;
-    bool mTestSubset;
-    bool mTestGeometry;
-    bool mTestGeometryExact;
-    bool mLoadGeometry;
+    IteratorMode mMode = FileScan;
+    long mNextId = 0;
+    bool mTestSubset = false;
+    bool mTestGeometry = false;
+    bool mTestGeometryExact = false;
+    bool mLoadGeometry = false;
 };
 
 

diff --git a/src/providers/gpx/qgsgpxfeatureiterator.cpp b/src/providers/gpx/qgsgpxfeatureiterator.cpp
@@ -27,7 +27,6 @@
 
 QgsGPXFeatureIterator::QgsGPXFeatureIterator( QgsGPXFeatureSource *source, bool ownSource, const QgsFeatureRequest &request )
   : QgsAbstractFeatureIteratorFromSource<QgsGPXFeatureSource>( source, ownSource, request )
-  , mFetchedFid( false )
 {
   rewind();
 }

diff --git a/src/providers/gpx/qgsgpxfeatureiterator.h b/src/providers/gpx/qgsgpxfeatureiterator.h
@@ -31,7 +31,7 @@ class QgsGPXFeatureSource : public QgsAbstractFeatureSource
 
     virtual QgsFeatureIterator getFeatures( const QgsFeatureRequest &request ) override;
 
-  protected:
+  private:
     QString mFileName;
     QgsGPXProvider::DataType mFeatureType;
     QgsGPSData *data = nullptr;
@@ -56,6 +56,8 @@ class QgsGPXFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsGPX
 
     virtual bool fetchFeature( QgsFeature &feature ) override;
 
+  private:
+
     bool readFid( QgsFeature &feature );
 
     bool readWaypoint( const QgsWaypoint &wpt, QgsFeature &feature );
@@ -70,16 +72,14 @@ class QgsGPXFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsGPX
     void readAttributes( QgsFeature &feature, const QgsRoute &rte );
     void readAttributes( QgsFeature &feature, const QgsTrack &trk );
 
-  protected:
-
     //! Current waypoint iterator
     QgsGPSData::WaypointIterator mWptIter;
     //! Current route iterator
     QgsGPSData::RouteIterator mRteIter;
     //! Current track iterator
     QgsGPSData::TrackIterator mTrkIter;
 
-    bool mFetchedFid;
+    bool mFetchedFid = false;
 };
 
 #endif // QGSGPXFEATUREITERATOR_H
diff --git a/src/providers/grass/qgsgrassfeatureiterator.cpp b/src/providers/grass/qgsgrassfeatureiterator.cpp
@@ -69,9 +69,6 @@ void copy_boxlist_and_destroy( struct boxlist *blist, struct ilist *list )
 
 QgsGrassFeatureIterator::QgsGrassFeatureIterator( QgsGrassFeatureSource *source, bool ownSource, const QgsFeatureRequest &request )
   : QgsAbstractFeatureIteratorFromSource<QgsGrassFeatureSource>( source, ownSource, request )
-  , mCanceled( false )
-  , mNextCidx( 0 )
-  , mNextLid( 1 )
 {
 
   // WARNING: the iterater cannot use mutex lock for its whole life, because QgsVectorLayerFeatureIterator is opening