[Spatialite] Fix crash on iterator closing if connection failed.

If an iterator fails to open the spatialite database (mHandle == nullptr in QgsSpatiaLiteFeatureIterator::QgsSpatiaLiteFeatureIterator() ), a crash will occur at the destruction of the QgsSpatiaLiteFeatureSource, due to the iterator not being removed from the list of active iterators. Currently QgsSpatiaLiteFeatureIterator::close() does not call iteratorClosed() if mHandle is invalid, which later causes QgsAbstractFeatureSource::~QgsAbstractFeatureSource() to try calling the close() method of a now defunct iterator. If not applying the patch, the added test case crashes with: 177: src/providers/spatialite/qgsspatialiteconnection.cpp: 736: (openDb) [1ms] New sqlite connection for /tmp/test.sqlite.corrupt 177: src/providers/spatialite/qgsspatialiteconnection.cpp: 750: (openDb) [1ms] Failure while connecting to: /tmp/test.sqlite.corrupt 177: 177: invalid metadata tables 177: src/core/qgsfeaturerequest.cpp: 259: (~QgsAbstractFeatureSource) [0ms] closing active iterator 177: CMake Error at PyQgsSpatialiteProvider.cmake:22 (MESSAGE): 177: Test failed: Segmentation fault (cherry picked from commit 2b15eaa)
qgis · Mar 24, 2016 · fb1aa6b · fb1aa6b
1 parent 2f7d4a7
commit fb1aa6b
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 1 deletion.
diff --git a/src/providers/spatialite/qgsspatialitefeatureiterator.cpp b/src/providers/spatialite/qgsspatialitefeatureiterator.cpp
@@ -235,11 +235,17 @@ bool QgsSpatiaLiteFeatureIterator::rewind()
 
 bool QgsSpatiaLiteFeatureIterator::close()
 {
-  if ( !mHandle )
+  if ( mClosed )
     return false;
 
   iteratorClosed();
 
+  if ( !mHandle )
+  {
+    mClosed = true;
+    return false;
+  }
+
   if ( sqliteStatement )
   {
     sqlite3_finalize( sqliteStatement );

diff --git a/tests/src/python/test_provider_spatialite.py b/tests/src/python/test_provider_spatialite.py
@@ -14,6 +14,7 @@
 
 import qgis
 import os
+import shutil
 import tempfile
 
 from qgis.core import QgsVectorLayer, QgsPoint, QgsFeature
@@ -202,6 +203,16 @@ def test_case(self):
         fields = [f.name() for f in l.dataProvider().fields()]
         assert('Geometry' not in fields)
 
+    def test_invalid_iterator(self):
+        """ Test invalid iterator """
+        corrupt_dbname = self.dbname + '.corrupt'
+        shutil.copy(self.dbname, corrupt_dbname)
+        layer = QgsVectorLayer("dbname=%s table=test_pg (geometry)" % corrupt_dbname, "test_pg", "spatialite")
+        # Corrupt the database
+        open(corrupt_dbname, 'wb').write('')
+        layer.getFeatures()
+        layer = None
+        os.unlink(corrupt_dbname)
 
 if __name__ == '__main__':
     unittest.main()