Skip to content

Commit

Permalink
[Spatialite provider] Make sure to release dangling connections on pr…
Browse files Browse the repository at this point in the history
…ovider closing

Fixes #15137
  • Loading branch information
rouault committed Jun 30, 2016
1 parent 55f172b commit c237ba7
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 5 deletions.
16 changes: 15 additions & 1 deletion src/providers/spatialite/qgsspatialiteconnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,10 @@ class QgsSqliteHandle
//
public:
QgsSqliteHandle( sqlite3 * handle, const QString& dbPath, bool shared )
: ref( shared ? 1 : -1 ), sqlite_handle( handle ), mDbPath( dbPath )
: ref( shared ? 1 : -1 )
, sqlite_handle( handle )
, mDbPath( dbPath )
, mIsValid( true )
{
}

Expand All @@ -145,6 +148,16 @@ class QgsSqliteHandle
return mDbPath;
}

bool isValid() const
{
return mIsValid;
}

void invalidate()
{
mIsValid = false;
}

//
// libsqlite3 wrapper
//
Expand All @@ -165,6 +178,7 @@ class QgsSqliteHandle
int ref;
sqlite3 *sqlite_handle;
QString mDbPath;
bool mIsValid;

static QMap < QString, QgsSqliteHandle * > handles;
};
Expand Down
8 changes: 5 additions & 3 deletions src/providers/spatialite/qgsspatialiteconnpool.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,15 @@ inline void qgsConnectionPool_ConnectionDestroy( QgsSqliteHandle* c )

inline void qgsConnectionPool_InvalidateConnection( QgsSqliteHandle* c )
{
Q_UNUSED( c );
/* Invalidation is used in particular by the WFS provider that uses a */
/* temporary spatialite DB and want to delete it at some point. For that */
/* it must invalidate all handles pointing to it */
c->invalidate();
}

inline bool qgsConnectionPool_ConnectionIsValid( QgsSqliteHandle* c )
{
Q_UNUSED( c );
return true;
return c->isValid();
}


Expand Down
1 change: 1 addition & 0 deletions src/providers/spatialite/qgsspatialiteprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,7 @@ QgsSpatiaLiteProvider::QgsSpatiaLiteProvider( QString const &uri )
QgsSpatiaLiteProvider::~QgsSpatiaLiteProvider()
{
closeDb();
invalidateConnections( mSqlitePath );
}

QgsAbstractFeatureSource* QgsSpatiaLiteProvider::featureSource() const
Expand Down
85 changes: 84 additions & 1 deletion tests/src/python/test_provider_spatialite.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import qgis
import os
import shutil
import sys
import tempfile

from qgis.core import QgsVectorLayer, QgsPoint, QgsFeature
Expand All @@ -36,10 +37,23 @@
start_app()
TEST_DATA_DIR = unitTestDataPath()


def die(error_message):
raise Exception(error_message)

def count_opened_filedescriptors(filename_to_test):
count = -1
if sys.platform.startswith('linux'):
count = 0
open_files_dirname = '/proc/%d/fd' % os.getpid()
filenames = os.listdir(open_files_dirname)
for filename in filenames:
full_filename = open_files_dirname + '/' + filename
if os.path.exists(full_filename):
link = os.readlink(full_filename)
if os.path.basename(link) == os.path.basename(filename_to_test):
count += 1
return count


class TestQgsSpatialiteProvider(unittest.TestCase, ProviderTestCase):

Expand Down Expand Up @@ -214,5 +228,74 @@ def test_invalid_iterator(self):
layer = None
os.unlink(corrupt_dbname)


def testNoDanglingFileDescriptorAfterCloseVariant1(self):
''' Test that when closing the provider all file handles are released '''

temp_dbname = self.dbname + '.no_dangling_test1'
shutil.copy(self.dbname, temp_dbname)

vl = QgsVectorLayer("dbname=%s table=test_n (geometry)" % temp_dbname, "test_n", "spatialite")
self.assertTrue(vl.isValid())
# The iterator will take one extra connection
myiter = vl.getFeatures()
print(vl.featureCount())
# Consume one feature but the iterator is still opened
f = next(myiter)
self.assertTrue(f.isValid())

if sys.platform.startswith('linux'):
self.assertEqual(count_opened_filedescriptors(temp_dbname), 2)

# does NO release one file descriptor, because shared with the iterator
del vl

# Non portable, but Windows testing is done with trying to unlink
if sys.platform.startswith('linux'):
self.assertEqual(count_opened_filedescriptors(temp_dbname), 2)

f = next(myiter)
self.assertTrue(f.isValid())

# Should release one file descriptor
del myiter

# Non portable, but Windows testing is done with trying to unlink
if sys.platform.startswith('linux'):
self.assertEqual(count_opened_filedescriptors(temp_dbname), 0)

# Check that deletion works well (can only fail on Windows)
os.unlink(temp_dbname)
self.assertFalse(os.path.exists(temp_dbname))

def testNoDanglingFileDescriptorAfterCloseVariant2(self):
''' Test that when closing the provider all file handles are released '''

temp_dbname = self.dbname + '.no_dangling_test2'
shutil.copy(self.dbname, temp_dbname)

vl = QgsVectorLayer("dbname=%s table=test_n (geometry)" % temp_dbname, "test_n", "spatialite")
self.assertTrue(vl.isValid())
self.assertTrue(vl.isValid())
# Consume all features.
myiter = vl.getFeatures()
for feature in myiter:
pass
# The iterator is closed
if sys.platform.startswith('linux'):
self.assertEqual(count_opened_filedescriptors(temp_dbname), 2)

# Should release one file descriptor
del vl

# Non portable, but Windows testing is done with trying to unlink
if sys.platform.startswith('linux'):
self.assertEqual(count_opened_filedescriptors(temp_dbname), 0)

# Check that deletion works well (can only fail on Windows)
os.unlink(temp_dbname)
self.assertFalse(os.path.exists(temp_dbname))


if __name__ == '__main__':
unittest.main()

0 comments on commit c237ba7

Please sign in to comment.