Skip to content

Commit

Permalink
Don't crash providers when destrucing with fake open connection
Browse files Browse the repository at this point in the history
  • Loading branch information
m-kuhn committed Dec 22, 2015
1 parent 73ba0e8 commit 3dc832a
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 8 deletions.
8 changes: 5 additions & 3 deletions src/core/qgsfeatureiterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ class QgsExpressionSorter
QgsAbstractFeatureIterator::QgsAbstractFeatureIterator( const QgsFeatureRequest& request )
: mRequest( request )
, mClosed( false )
, mZombie( false )
, refs( 0 )
, mFetchedCount( 0 )
, mGeometrySimplifier( nullptr )
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
}

Expand Down
12 changes: 11 additions & 1 deletion src/core/qgsfeatureiterator.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 )
Expand Down
6 changes: 4 additions & 2 deletions src/providers/ogr/qgsogrfeatureiterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ bool QgsOgrFeatureIterator::rewind()

bool QgsOgrFeatureIterator::close()
{
if ( mClosed )
if ( !mConn )
return false;

iteratorClosed();
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/providers/postgres/qgspostgresfeatureiterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ bool QgsPostgresFeatureIterator::rewind()

bool QgsPostgresFeatureIterator::close()
{
if ( mClosed )
if ( !mConn )
return false;

mConn->closeCursor( mCursorName );
Expand Down
2 changes: 1 addition & 1 deletion src/providers/spatialite/qgsspatialitefeatureiterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ bool QgsSpatiaLiteFeatureIterator::rewind()

bool QgsSpatiaLiteFeatureIterator::close()
{
if ( mClosed )
if ( !mHandle )
return false;

iteratorClosed();
Expand Down

0 comments on commit 3dc832a

Please sign in to comment.