Skip to content

Commit 0497e4a

Browse files
committedSep 23, 2016
Fix database locking when editing GeoPackage
Concurrent read and write can lock a GeoPackage database given the default journaling mode of SQLite (delete). Use WAL when possible to avoid that. Fixes #15351
1 parent dfeb663 commit 0497e4a

File tree

5 files changed

+237
-3
lines changed

5 files changed

+237
-3
lines changed
 

‎src/providers/ogr/qgsogrconnpool.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#define QGSOGRCONNPOOL_H
1818

1919
#include "qgsconnectionpool.h"
20+
#include "qgsogrprovider.h"
2021
#include <ogr_api.h>
2122

2223

@@ -43,7 +44,7 @@ inline void qgsConnectionPool_ConnectionCreate( QString connInfo, QgsOgrConn*& c
4344

4445
inline void qgsConnectionPool_ConnectionDestroy( QgsOgrConn* c )
4546
{
46-
OGR_DS_Destroy( c->ds );
47+
QgsOgrProviderUtils::OGRDestroyWrapper( c->ds );
4748
delete c;
4849
}
4950

‎src/providers/ogr/qgsogrfeatureiterator.cpp

+7
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,12 @@ bool QgsOgrFeatureIterator::close()
254254

255255
iteratorClosed();
256256

257+
// Will for example release SQLite3 statements
258+
if ( ogrLayer )
259+
{
260+
OGR_L_ResetReading( ogrLayer );
261+
}
262+
257263
if ( mSubsetStringSet )
258264
{
259265
OGR_DS_ReleaseResultSet( mConn->ds, ogrLayer );
@@ -263,6 +269,7 @@ bool QgsOgrFeatureIterator::close()
263269
QgsOgrConnPool::instance()->releaseConnection( mConn );
264270

265271
mConn = nullptr;
272+
ogrLayer = nullptr;
266273

267274
mClosed = true;
268275
return true;

‎src/providers/ogr/qgsogrprovider.cpp

+146-2
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,13 @@ email : sherman at mrcc.com
4949
#include "qgsvectorlayerimport.h"
5050
#include "qgslocalec.h"
5151

52+
#ifdef Q_OS_WIN
53+
#include <windows.h>
54+
#endif
55+
#ifdef Q_OS_LINUX
56+
#include <sys/vfs.h>
57+
#endif
58+
5259
static const QString TEXT_PROVIDER_KEY = "ogr";
5360
static const QString TEXT_PROVIDER_DESCRIPTION =
5461
QString( "OGR data provider" )
@@ -384,11 +391,14 @@ QgsOgrProvider::QgsOgrProvider( QString const & uri )
384391

385392
QgsOgrProvider::~QgsOgrProvider()
386393
{
387-
close();
388394
QgsOgrConnPool::instance()->unref( dataSourceUri() );
389395
// We must also make sure to flush unusef cached connections so that
390396
// the file can be removed (#15137)
391397
QgsOgrConnPool::instance()->invalidateConnections( dataSourceUri() );
398+
399+
// Do that as last step for final cleanup that might be prevented by
400+
// still opened datasets.
401+
close();
392402
}
393403

394404
QgsAbstractFeatureSource* QgsOgrProvider::featureSource() const
@@ -2905,6 +2915,125 @@ OGRDataSourceH QgsOgrProviderUtils::OGROpenWrapper( const char* pszPath, bool bU
29052915
return hDS;
29062916
}
29072917

2918+
static bool IsLocalFile( const QString& path )
2919+
{
2920+
QString dirName( QFileInfo( path ).absolutePath() );
2921+
// Start with the OS specific methods since the QT >= 5.4 method just
2922+
// return a string and not an enumerated type.
2923+
#if defined(Q_OS_WIN)
2924+
if ( dirName.startsWith( "\\\\" ) )
2925+
return false;
2926+
if ( dirName.length() >= 3 && dirName[1] == ':' &&
2927+
( dirName[2] == '\\' || dirName[2] == '/' ) )
2928+
{
2929+
dirName.resize( 3 );
2930+
return GetDriveType( dirName.toAscii().constData() ) != DRIVE_REMOTE;
2931+
}
2932+
return true;
2933+
#elif defined(Q_OS_LINUX)
2934+
struct statfs sStatFS;
2935+
if ( statfs( dirName.toAscii().constData(), &sStatFS ) == 0 )
2936+
{
2937+
// Codes from http://man7.org/linux/man-pages/man2/statfs.2.html
2938+
if ( sStatFS.f_type == 0x6969 /* NFS */ ||
2939+
sStatFS.f_type == 0x517b /* SMB */ ||
2940+
sStatFS.f_type == 0xff534d42 /* CIFS */ )
2941+
{
2942+
return false;
2943+
}
2944+
}
2945+
return true;
2946+
#elif QT_VERSION >= QT_VERSION_CHECK(5, 4, 0)
2947+
QStorageInfo info( dirName );
2948+
QString fileSystem( info.fileSystemType() );
2949+
QgsDebugMsg( QString( "Filesystem for %1 is %2" ).arg( path ).arg( fileSystem ) );
2950+
return path != "nfs" && path != "smbfs";
2951+
#else
2952+
return true;
2953+
#endif
2954+
}
2955+
2956+
void QgsOgrProviderUtils::OGRDestroyWrapper( OGRDataSourceH ogrDataSource )
2957+
{
2958+
if ( !ogrDataSource )
2959+
return;
2960+
OGRSFDriverH ogrDriver = OGR_DS_GetDriver( ogrDataSource );
2961+
QString ogrDriverName = OGR_Dr_GetName( ogrDriver );
2962+
QString datasetName( FROM8( OGR_DS_GetName( ogrDataSource ) ) );
2963+
if ( ogrDriverName == "GPKG" &&
2964+
IsLocalFile( datasetName ) &&
2965+
!CPLGetConfigOption( "OGR_SQLITE_JOURNAL", NULL ) )
2966+
{
2967+
// We need to reset all iterators on layers, otherwise we will not
2968+
// be able to change journal_mode.
2969+
int layerCount = OGR_DS_GetLayerCount( ogrDataSource );
2970+
for ( int i = 0; i < layerCount; i ++ )
2971+
{
2972+
OGR_L_ResetReading( OGR_DS_GetLayer( ogrDataSource, i ) );
2973+
}
2974+
2975+
CPLPushErrorHandler( CPLQuietErrorHandler );
2976+
QgsDebugMsg( "GPKG: Trying to return to delete mode" );
2977+
bool bSuccess = false;
2978+
OGRLayerH hSqlLyr = OGR_DS_ExecuteSQL( ogrDataSource,
2979+
"PRAGMA journal_mode = delete",
2980+
NULL, NULL );
2981+
if ( hSqlLyr != NULL )
2982+
{
2983+
OGRFeatureH hFeat = OGR_L_GetNextFeature( hSqlLyr );
2984+
if ( hFeat != NULL )
2985+
{
2986+
const char* pszRet = OGR_F_GetFieldAsString( hFeat, 0 );
2987+
bSuccess = EQUAL( pszRet, "delete" );
2988+
QgsDebugMsg( QString( "Return: %1" ).arg( pszRet ) );
2989+
OGR_F_Destroy( hFeat );
2990+
}
2991+
}
2992+
else if ( CPLGetLastErrorType() != CE_None )
2993+
{
2994+
QgsDebugMsg( QString( "Return: %1" ).arg( CPLGetLastErrorMsg() ) );
2995+
}
2996+
OGR_DS_ReleaseResultSet( ogrDataSource, hSqlLyr );
2997+
CPLPopErrorHandler();
2998+
OGR_DS_Destroy( ogrDataSource );
2999+
3000+
// This may have not worked if the file was opened in read-only mode,
3001+
// so retry in update mode
3002+
if ( !bSuccess )
3003+
{
3004+
QgsDebugMsg( "GPKG: Trying again" );
3005+
CPLSetThreadLocalConfigOption( "OGR_SQLITE_JOURNAL", "DELETE" );
3006+
ogrDataSource = OGROpen( TO8F( datasetName ), TRUE, NULL );
3007+
CPLSetThreadLocalConfigOption( "OGR_SQLITE_JOURNAL", NULL );
3008+
if ( ogrDataSource )
3009+
{
3010+
#ifdef QGISDEBUG
3011+
CPLPushErrorHandler( CPLQuietErrorHandler );
3012+
OGRLayerH hSqlLyr = OGR_DS_ExecuteSQL( ogrDataSource,
3013+
"PRAGMA journal_mode",
3014+
NULL, NULL );
3015+
CPLPopErrorHandler();
3016+
if ( hSqlLyr != NULL )
3017+
{
3018+
OGRFeatureH hFeat = OGR_L_GetNextFeature( hSqlLyr );
3019+
if ( hFeat != NULL )
3020+
{
3021+
const char* pszRet = OGR_F_GetFieldAsString( hFeat, 0 );
3022+
QgsDebugMsg( QString( "Return: %1" ).arg( pszRet ) );
3023+
OGR_F_Destroy( hFeat );
3024+
}
3025+
OGR_DS_ReleaseResultSet( ogrDataSource, hSqlLyr );
3026+
}
3027+
#endif
3028+
OGR_DS_Destroy( ogrDataSource );
3029+
}
3030+
}
3031+
}
3032+
else
3033+
{
3034+
OGR_DS_Destroy( ogrDataSource );
3035+
}
3036+
}
29083037

29093038
QByteArray QgsOgrProviderUtils::quotedIdentifier( QByteArray field, const QString& ogrDriverName )
29103039
{
@@ -3140,7 +3269,22 @@ void QgsOgrProvider::open( OpenMode mode )
31403269

31413270
// first try to open in update mode (unless specified otherwise)
31423271
if ( !openReadOnly )
3272+
{
3273+
if ( QFileInfo( mFilePath ).suffix().compare( "gpkg", Qt::CaseInsensitive ) == 0 &&
3274+
IsLocalFile( mFilePath ) &&
3275+
!CPLGetConfigOption( "OGR_SQLITE_JOURNAL", NULL ) &&
3276+
QSettings().value( "/qgis/walForSqlite3", true ).toBool() )
3277+
{
3278+
// For GeoPackage, we force opening of the file in WAL (Write Ahead Log)
3279+
// mode so as to avoid readers blocking writer(s), and vice-versa.
3280+
// https://www.sqlite.org/wal.html
3281+
// But only do that on a local file since WAL is advertized not to work
3282+
// on network shares
3283+
CPLSetThreadLocalConfigOption( "OGR_SQLITE_JOURNAL", "WAL" );
3284+
}
31433285
ogrDataSource = QgsOgrProviderUtils::OGROpenWrapper( TO8F( mFilePath ), true, &ogrDriver );
3286+
CPLSetThreadLocalConfigOption( "OGR_SQLITE_JOURNAL", NULL );
3287+
}
31443288

31453289
mValid = false;
31463290
if ( ogrDataSource )
@@ -3270,7 +3414,7 @@ void QgsOgrProvider::close()
32703414

32713415
if ( ogrDataSource )
32723416
{
3273-
OGR_DS_Destroy( ogrDataSource );
3417+
QgsOgrProviderUtils::OGRDestroyWrapper( ogrDataSource );
32743418
}
32753419
ogrDataSource = nullptr;
32763420
ogrLayer = nullptr;

‎src/providers/ogr/qgsogrprovider.h

+6
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ email : sherman at mrcc.com
1515
* *
1616
***************************************************************************/
1717

18+
#ifndef QGSOGRPROVIDER_H
19+
#define QGSOGRPROVIDER_H
20+
1821
#include "QTextCodec"
1922

2023
#include "qgsrectangle.h"
@@ -393,4 +396,7 @@ class QgsOgrProviderUtils
393396
static QString quotedValue( const QVariant& value );
394397

395398
static OGRDataSourceH OGROpenWrapper( const char* pszPath, bool bUpdate, OGRSFDriverH *phDriver );
399+
static void OGRDestroyWrapper( OGRDataSourceH ogrDataSource );
396400
};
401+
402+
#endif // QGSOGRPROVIDER_H

‎tests/src/python/test_provider_ogr_gpkg.py

+76
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,15 @@ def GDAL_COMPUTE_VERSION(maj, min, rev):
3131
return ((maj) * 1000000 + (min) * 10000 + (rev) * 100)
3232

3333

34+
class ErrorReceiver():
35+
36+
def __init__(self):
37+
self.msg = None
38+
39+
def receiveError(self, msg):
40+
self.msg = msg
41+
42+
3443
class TestPyQgsOGRProviderGpkg(unittest.TestCase):
3544

3645
@classmethod
@@ -80,5 +89,72 @@ def testCurveGeometryType(self):
8089
# The geometries must be binarily identical
8190
self.assertEqual(got_geom.asWkb(), reference.asWkb(), 'Expected {}, got {}'.format(reference.exportToWkt(), got_geom.exportToWkt()))
8291

92+
def internalTestBug15351(self, orderClosing):
93+
94+
tmpfile = os.path.join(self.basetestpath, 'testBug15351.gpkg')
95+
ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile)
96+
lyr = ds.CreateLayer('test', geom_type=ogr.wkbPoint)
97+
f = ogr.Feature(lyr.GetLayerDefn())
98+
f.SetGeometry(ogr.CreateGeometryFromWkt('POINT(0 0)'))
99+
lyr.CreateFeature(f)
100+
f = None
101+
ds = None
102+
103+
vl = QgsVectorLayer(u'{}'.format(tmpfile), u'test', u'ogr')
104+
self.assertTrue(vl.startEditing())
105+
self.assertTrue(vl.changeGeometry(1, QgsGeometry.fromWkt('Point (3 50)')))
106+
107+
# Iterate over features (will open a new OGR connection), but do not
108+
# close the iterator for now
109+
it = vl.getFeatures()
110+
f = QgsFeature()
111+
it.nextFeature(f)
112+
113+
if orderClosing == 'closeIter_commit_closeProvider':
114+
it = None
115+
116+
# Commit changes
117+
cbk = ErrorReceiver()
118+
vl.dataProvider().raiseError.connect(cbk.receiveError)
119+
self.assertTrue(vl.commitChanges())
120+
self.assertIsNone(cbk.msg)
121+
122+
# Close layer and iterator in different orders
123+
if orderClosing == 'closeIter_commit_closeProvider':
124+
vl = None
125+
elif orderClosing == 'commit_closeProvider_closeIter':
126+
vl = None
127+
it = None
128+
else:
129+
assert orderClosing == 'commit_closeIter_closeProvider'
130+
it = None
131+
vl = None
132+
133+
# Test that we succeeded restoring default journal mode, and we
134+
# are not let in WAL mode.
135+
ds = ogr.Open(tmpfile)
136+
lyr = ds.ExecuteSQL('PRAGMA journal_mode')
137+
f = lyr.GetNextFeature()
138+
res = f.GetField(0)
139+
ds.ReleaseResultSet(lyr)
140+
ds = None
141+
self.assertEqual(res, 'delete')
142+
143+
@unittest.expectedFailure(int(gdal.VersionInfo('VERSION_NUM')) < GDAL_COMPUTE_VERSION(2, 0, 0))
144+
# We need GDAL 2.0 to issue PRAGMA journal_mode
145+
# Note: for that case, we don't strictly need turning on WAL
146+
def testBug15351_closeIter_commit_closeProvider(self):
147+
self.internalTestBug15351('closeIter_commit_closeProvider')
148+
149+
@unittest.expectedFailure(int(gdal.VersionInfo('VERSION_NUM')) < GDAL_COMPUTE_VERSION(2, 0, 0))
150+
# We need GDAL 2.0 to issue PRAGMA journal_mode
151+
def testBug15351_closeIter_commit_closeProvider(self):
152+
self.internalTestBug15351('closeIter_commit_closeProvider')
153+
154+
@unittest.expectedFailure(int(gdal.VersionInfo('VERSION_NUM')) < GDAL_COMPUTE_VERSION(2, 0, 0))
155+
# We need GDAL 2.0 to issue PRAGMA journal_mode
156+
def testBug15351_commit_closeIter_closeProvider(self):
157+
self.internalTestBug15351('commit_closeIter_closeProvider')
158+
83159
if __name__ == '__main__':
84160
unittest.main()

0 commit comments

Comments
 (0)
Please sign in to comment.