Skip to content

Commit 55f172b

Browse files
committed
[OGR provider] Make sure to release dangling connections on provider closing
Fixes #15137
1 parent 6076451 commit 55f172b

File tree

2 files changed

+89
-0
lines changed

2 files changed

+89
-0
lines changed

src/providers/ogr/qgsogrprovider.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,9 @@ QgsOgrProvider::~QgsOgrProvider()
385385
{
386386
close();
387387
QgsOgrConnPool::instance()->unref( dataSourceUri() );
388+
// We must also make sure to flush unusef cached connections so that
389+
// the file can be removed (#15137)
390+
QgsOgrConnPool::instance()->invalidateConnections( dataSourceUri() );
388391
}
389392

390393
QgsAbstractFeatureSource* QgsOgrProvider::featureSource() const

tests/src/python/test_provider_ogr.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import os
1616
import shutil
17+
import sys
1718
import tempfile
1819

1920
from qgis.core import QgsVectorLayer, QgsVectorDataProvider
@@ -29,6 +30,21 @@
2930
# Note - doesn't implement ProviderTestCase as most OGR provider is tested by the shapefile provider test
3031

3132

33+
def count_opened_filedescriptors(filename_to_test):
34+
count = -1
35+
if sys.platform.startswith('linux'):
36+
count = 0
37+
open_files_dirname = '/proc/%d/fd' % os.getpid()
38+
filenames = os.listdir(open_files_dirname)
39+
for filename in filenames:
40+
full_filename = open_files_dirname + '/' + filename
41+
if os.path.exists(full_filename):
42+
link = os.readlink(full_filename)
43+
if os.path.basename(link) == os.path.basename(filename_to_test):
44+
count += 1
45+
return count
46+
47+
3248
class PyQgsOGRProvider(unittest.TestCase):
3349

3450
@classmethod
@@ -66,6 +82,76 @@ def testUpdateMode(self):
6682
self.assertTrue(vl.dataProvider().leaveUpdateMode())
6783
self.assertEqual(vl.dataProvider().property("_debug_open_mode"), "read-write")
6884

85+
def testNoDanglingFileDescriptorAfterCloseVariant1(self):
86+
''' Test that when closing the provider all file handles are released '''
87+
88+
datasource = os.path.join(self.basetestpath, 'testNoDanglingFileDescriptorAfterCloseVariant1.csv')
89+
with open(datasource, 'wt') as f:
90+
f.write('id,WKT\n')
91+
f.write('1,\n')
92+
f.write('2,POINT(2 49)\n')
93+
94+
vl = QgsVectorLayer(u'{}|layerid=0'.format(datasource), u'test', u'ogr')
95+
self.assertTrue(vl.isValid())
96+
# The iterator will take one extra connection
97+
myiter = vl.getFeatures()
98+
# Consume one feature but the iterator is still opened
99+
f = next(myiter)
100+
self.assertTrue(f.isValid())
101+
102+
if sys.platform.startswith('linux'):
103+
self.assertEqual(count_opened_filedescriptors(datasource), 2)
104+
105+
# Should release one file descriptor
106+
del vl
107+
108+
# Non portable, but Windows testing is done with trying to unlink
109+
if sys.platform.startswith('linux'):
110+
self.assertEqual(count_opened_filedescriptors(datasource), 1)
111+
112+
f = next(myiter)
113+
self.assertTrue(f.isValid())
114+
115+
# Should release one file descriptor
116+
del myiter
117+
118+
# Non portable, but Windows testing is done with trying to unlink
119+
if sys.platform.startswith('linux'):
120+
self.assertEqual(count_opened_filedescriptors(datasource), 0)
121+
122+
# Check that deletion works well (can only fail on Windows)
123+
os.unlink(datasource)
124+
self.assertFalse(os.path.exists(datasource))
125+
126+
def testNoDanglingFileDescriptorAfterCloseVariant2(self):
127+
''' Test that when closing the provider all file handles are released '''
128+
129+
datasource = os.path.join(self.basetestpath, 'testNoDanglingFileDescriptorAfterCloseVariant2.csv')
130+
with open(datasource, 'wt') as f:
131+
f.write('id,WKT\n')
132+
f.write('1,\n')
133+
f.write('2,POINT(2 49)\n')
134+
135+
vl = QgsVectorLayer(u'{}|layerid=0'.format(datasource), u'test', u'ogr')
136+
self.assertTrue(vl.isValid())
137+
# Consume all features.
138+
myiter = vl.getFeatures()
139+
for feature in myiter:
140+
pass
141+
# The iterator is closed, but the corresponding connection still not closed
142+
if sys.platform.startswith('linux'):
143+
self.assertEqual(count_opened_filedescriptors(datasource), 2)
144+
145+
# Should release one file descriptor
146+
del vl
147+
148+
# Non portable, but Windows testing is done with trying to unlink
149+
if sys.platform.startswith('linux'):
150+
self.assertEqual(count_opened_filedescriptors(datasource), 0)
151+
152+
# Check that deletion works well (can only fail on Windows)
153+
os.unlink(datasource)
154+
self.assertFalse(os.path.exists(datasource))
69155

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

0 commit comments

Comments
 (0)