Don't crash providers when destrucing with fake open connection

src/core/qgsfeatureiterator.cpp

@@ -120,6 +120,7 @@ class QgsExpressionSorter
 QgsAbstractFeatureIterator::QgsAbstractFeatureIterator( const QgsFeatureRequest& request )
     : mRequest( request )
     , mClosed( false )
+    , mZombie( false )
     , refs( 0 )
     , mFetchedCount( 0 )
     , mGeometrySimplifier( nullptr )
@@ -152,8 +153,8 @@ bool QgsAbstractFeatureIterator::nextFeature( QgsFeature& f )
     else
     {
       dataOk = false;
-      // don't call close, the provider connection has already been closed
-      mClosed = true;
+      // even the zombie dies at this point...
+      mZombie = false;
     }
   }
   else
@@ -289,7 +290,8 @@ void QgsAbstractFeatureIterator::setupOrderBy( const QList<QgsFeatureRequest::Or
 
     mFeatureIterator = mCachedFeatures.constBegin();
     mUseCachedFeatures = true;
-    mClosed = false;
+    // The real iterator is closed, we are only serving cached features
+    mZombie = true;
   }
 }
 

src/core/qgsfeatureiterator.h

@@ -89,7 +89,17 @@ class CORE_EXPORT QgsAbstractFeatureIterator
     /** Set to true, as soon as the iterator is closed. */
     bool mClosed;
 
+    /**
+     * A feature iterator may be closed already but still be serving features from the cache.
+     * This is done when we serve features which have been pre-fetched and the order by has
+     * been locally sorted.
+     * In such a scenario, all resources have been released (mClosed is true) but the deads
+     * are still alive.
+     */
+    bool mZombie;
+
     //! reference counting (to allow seamless copying of QgsFeatureIterator instances)
+    //! TODO QGIS3: make this private
     int refs;
     void ref(); //!< add reference
     void deref(); //!< remove reference, delete if refs == 0
@@ -247,7 +257,7 @@ inline bool QgsFeatureIterator::close()
 
 inline bool QgsFeatureIterator::isClosed() const
 {
-  return mIter ? mIter->mClosed : true;
+  return mIter ? mIter->mClosed && !mIter->mZombie : true;
 }
 
 inline bool operator== ( const QgsFeatureIterator &fi1, const QgsFeatureIterator &fi2 )

src/providers/ogr/qgsogrfeatureiterator.cpp

@@ -243,7 +243,7 @@ bool QgsOgrFeatureIterator::rewind()
 
 bool QgsOgrFeatureIterator::close()
 {
-  if ( mClosed )
+  if ( !mConn )
     return false;
 
   iteratorClosed();
@@ -253,7 +253,9 @@ bool QgsOgrFeatureIterator::close()
     OGR_DS_ReleaseResultSet( mConn->ds, ogrLayer );
   }
 
-  QgsOgrConnPool::instance()->releaseConnection( mConn );
+  if ( mConn )
+    QgsOgrConnPool::instance()->releaseConnection( mConn );
+
   mConn = nullptr;
 
   mClosed = true;

src/providers/postgres/qgspostgresfeatureiterator.cpp

@@ -313,7 +313,7 @@ bool QgsPostgresFeatureIterator::rewind()
 
 bool QgsPostgresFeatureIterator::close()
 {
-  if ( mClosed )
+  if ( !mConn )
     return false;
 
   mConn->closeCursor( mCursorName );

src/providers/spatialite/qgsspatialitefeatureiterator.cpp

@@ -235,7 +235,7 @@ bool QgsSpatiaLiteFeatureIterator::rewind()
 
 bool QgsSpatiaLiteFeatureIterator::close()
 {
-  if ( mClosed )
+  if ( !mHandle )
     return false;
 
   iteratorClosed();