Skip to content

Commit

Permalink
Fix database locking when editing GeoPackage
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rouault committed Sep 23, 2016
1 parent bcec7ab commit f939e9c
Show file tree
Hide file tree
Showing 5 changed files with 355 additions and 4 deletions.
3 changes: 2 additions & 1 deletion src/providers/ogr/qgsogrconnpool.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#define QGSOGRCONNPOOL_H

#include "qgsconnectionpool.h"
#include "qgsogrprovider.h"
#include <ogr_api.h>


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

inline void qgsConnectionPool_ConnectionDestroy( QgsOgrConn* c )
{
OGR_DS_Destroy( c->ds );
QgsOgrUtils::OGRDestroyWrapper( c->ds );
delete c;
}

Expand Down
7 changes: 7 additions & 0 deletions src/providers/ogr/qgsogrfeatureiterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,12 @@ bool QgsOgrFeatureIterator::close()

iteratorClosed();

// Will for example release SQLite3 statements
if ( ogrLayer )
{
OGR_L_ResetReading( ogrLayer );
}

if ( mSubsetStringSet )
{
OGR_DS_ReleaseResultSet( mConn->ds, ogrLayer );
Expand All @@ -310,6 +316,7 @@ bool QgsOgrFeatureIterator::close()
QgsOgrConnPool::instance()->releaseConnection( mConn );

mConn = nullptr;
ogrLayer = nullptr;

mClosed = true;
return true;
Expand Down
177 changes: 174 additions & 3 deletions src/providers/ogr/qgsogrprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ email : sherman at mrcc.com
#include "qgsvectorlayerimport.h"
#include "qgslocalec.h"

#ifdef Q_OS_WIN
#include <windows.h>
#endif
#ifdef Q_OS_LINUX
#include <sys/vfs.h>
#endif

static const QString TEXT_PROVIDER_KEY = "ogr";
static const QString TEXT_PROVIDER_DESCRIPTION =
QString( "OGR data provider" )
Expand Down Expand Up @@ -383,11 +390,14 @@ QgsOgrProvider::QgsOgrProvider( QString const & uri )

QgsOgrProvider::~QgsOgrProvider()
{
close();
QgsOgrConnPool::instance()->unref( dataSourceUri() );
// We must also make sure to flush unusef cached connections so that
// the file can be removed (#15137)
QgsOgrConnPool::instance()->invalidateConnections( dataSourceUri() );

// Do that as last step for final cleanup that might be prevented by
// still opened datasets.
close();
}

QgsAbstractFeatureSource* QgsOgrProvider::featureSource() const
Expand Down Expand Up @@ -2627,6 +2637,152 @@ void QgsOgrProvider::forceReload()
QgsOgrConnPool::instance()->invalidateConnections( dataSourceUri() );
}

OGRDataSourceH QgsOgrUtils::OGROpenWrapper( const char* pszPath, bool bUpdate, OGRSFDriverH *phDriver )
{
CPLErrorReset();
OGRSFDriverH hDriver = nullptr;
OGRDataSourceH hDS = OGROpen( pszPath, bUpdate, &hDriver );
if ( phDriver )
*phDriver = hDriver;
if ( !hDS )
return nullptr;
// GDAL < 1.11.5 has a crashing bug with GeoPackage databases with curve geometry
// types (https://trac.osgeo.org/gdal/ticket/6558)
#if GDAL_VERSION_MAJOR == 1 && GDAL_VERSION_MINOR == 11 && GDAL_VERSION_MACRO < 5
const char* pszLastErrorMsg = CPLGetLastErrorMsg();
if ( hDriver == OGRGetDriverByName( "GPKG" ) &&
strstr( pszLastErrorMsg, "geometry column" ) &&
strstr( pszLastErrorMsg, "of type" ) &&
strstr( pszLastErrorMsg, "ignored" ) )
{
QgsDebugMsg( QString( "Ignoring %1 that is a GeoPackage DB with curve geometries" ).arg( pszPath ) );
OGR_DS_Destroy( hDS );
hDS = nullptr;
}
#endif
return hDS;
}

static bool IsLocalFile( const QString& path )
{
QString dirName( QFileInfo( path ).absolutePath() );
// Start with the OS specific methods since the QT >= 5.4 method just
// return a string and not an enumerated type.
#if defined(Q_OS_WIN)
if ( dirName.startsWith( "\\\\" ) )
return false;
if ( dirName.length() >= 3 && dirName[1] == ':' &&
( dirName[2] == '\\' || dirName[2] == '/' ) )
{
dirName.resize( 3 );
return GetDriveType( dirName.toAscii().constData() ) != DRIVE_REMOTE;
}
return true;
#elif defined(Q_OS_LINUX)
struct statfs sStatFS;
if ( statfs( dirName.toAscii().constData(), &sStatFS ) == 0 )
{
// Codes from http://man7.org/linux/man-pages/man2/statfs.2.html
if ( sStatFS.f_type == 0x6969 /* NFS */ ||
sStatFS.f_type == 0x517b /* SMB */ ||
sStatFS.f_type == 0xff534d42 /* CIFS */ )
{
return false;
}
}
return true;
#elif QT_VERSION >= QT_VERSION_CHECK(5, 4, 0)
QStorageInfo info( dirName );
QString fileSystem( info.fileSystemType() );
QgsDebugMsg( QString( "Filesystem for %1 is %2" ).arg( path ).arg( fileSystem ) );
return path != "nfs" && path != "smbfs";
#else
return true;
#endif
}

void QgsOgrUtils::OGRDestroyWrapper( OGRDataSourceH ogrDataSource )
{
if ( !ogrDataSource )
return;
OGRSFDriverH ogrDriver = OGR_DS_GetDriver( ogrDataSource );
QString ogrDriverName = OGR_Dr_GetName( ogrDriver );
QString datasetName( FROM8( OGR_DS_GetName( ogrDataSource ) ) );
if ( ogrDriverName == "GPKG" &&
IsLocalFile( datasetName ) &&
!CPLGetConfigOption( "OGR_SQLITE_JOURNAL", NULL ) )
{
// We need to reset all iterators on layers, otherwise we will not
// be able to change journal_mode.
int layerCount = OGR_DS_GetLayerCount( ogrDataSource );
for ( int i = 0; i < layerCount; i ++ )
{
OGR_L_ResetReading( OGR_DS_GetLayer( ogrDataSource, i ) );
}

CPLPushErrorHandler( CPLQuietErrorHandler );
QgsDebugMsg( "GPKG: Trying to return to delete mode" );
bool bSuccess = false;
OGRLayerH hSqlLyr = OGR_DS_ExecuteSQL( ogrDataSource,
"PRAGMA journal_mode = delete",
NULL, NULL );
if ( hSqlLyr != NULL )
{
OGRFeatureH hFeat = OGR_L_GetNextFeature( hSqlLyr );
if ( hFeat != NULL )
{
const char* pszRet = OGR_F_GetFieldAsString( hFeat, 0 );
bSuccess = EQUAL( pszRet, "delete" );
QgsDebugMsg( QString( "Return: %1" ).arg( pszRet ) );
OGR_F_Destroy( hFeat );
}
}
else if ( CPLGetLastErrorType() != CE_None )
{
QgsDebugMsg( QString( "Return: %1" ).arg( CPLGetLastErrorMsg() ) );
}
OGR_DS_ReleaseResultSet( ogrDataSource, hSqlLyr );
CPLPopErrorHandler();
OGR_DS_Destroy( ogrDataSource );

// This may have not worked if the file was opened in read-only mode,
// so retry in update mode
if ( !bSuccess )
{
QgsDebugMsg( "GPKG: Trying again" );
CPLSetThreadLocalConfigOption( "OGR_SQLITE_JOURNAL", "DELETE" );
ogrDataSource = OGROpen( TO8F( datasetName ), TRUE, NULL );
CPLSetThreadLocalConfigOption( "OGR_SQLITE_JOURNAL", NULL );
if ( ogrDataSource )
{
#ifdef QGISDEBUG
CPLPushErrorHandler( CPLQuietErrorHandler );
OGRLayerH hSqlLyr = OGR_DS_ExecuteSQL( ogrDataSource,
"PRAGMA journal_mode",
NULL, NULL );
CPLPopErrorHandler();
if ( hSqlLyr != NULL )
{
OGRFeatureH hFeat = OGR_L_GetNextFeature( hSqlLyr );
if ( hFeat != NULL )
{
const char* pszRet = OGR_F_GetFieldAsString( hFeat, 0 );
QgsDebugMsg( QString( "Return: %1" ).arg( pszRet ) );
OGR_F_Destroy( hFeat );
}
OGR_DS_ReleaseResultSet( ogrDataSource, hSqlLyr );
}
#endif
OGR_DS_Destroy( ogrDataSource );
}
}
}
else
{
OGR_DS_Destroy( ogrDataSource );
}
}

QByteArray QgsOgrUtils::quotedIdentifier( QByteArray field, const QString& ogrDriverName )
{
if ( ogrDriverName == "MySQL" )
Expand Down Expand Up @@ -2855,7 +3011,22 @@ void QgsOgrProvider::open( OpenMode mode )

// first try to open in update mode (unless specified otherwise)
if ( !openReadOnly )
ogrDataSource = OGROpen( TO8F( mFilePath ), true, &ogrDriver );
{
if ( QFileInfo( mFilePath ).suffix().compare( "gpkg", Qt::CaseInsensitive ) == 0 &&
IsLocalFile( mFilePath ) &&
!CPLGetConfigOption( "OGR_SQLITE_JOURNAL", NULL ) &&
QSettings().value( "/qgis/walForSqlite3", true ).toBool() )
{
// For GeoPackage, we force opening of the file in WAL (Write Ahead Log)
// mode so as to avoid readers blocking writer(s), and vice-versa.
// https://www.sqlite.org/wal.html
// But only do that on a local file since WAL is advertized not to work
// on network shares
CPLSetThreadLocalConfigOption( "OGR_SQLITE_JOURNAL", "WAL" );
}
ogrDataSource = QgsOgrUtils::OGROpenWrapper( TO8F( mFilePath ), true, &ogrDriver );
CPLSetThreadLocalConfigOption( "OGR_SQLITE_JOURNAL", NULL );
}

mValid = false;
if ( ogrDataSource )
Expand Down Expand Up @@ -2985,7 +3156,7 @@ void QgsOgrProvider::close()

if ( ogrDataSource )
{
OGR_DS_Destroy( ogrDataSource );
QgsOgrUtils::OGRDestroyWrapper( ogrDataSource );
}
ogrDataSource = nullptr;
ogrLayer = nullptr;
Expand Down
8 changes: 8 additions & 0 deletions src/providers/ogr/qgsogrprovider.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ email : sherman at mrcc.com
* *
***************************************************************************/

#ifndef QGSOGRPROVIDER_H
#define QGSOGRPROVIDER_H

#include "QTextCodec"

#include "qgsrectangle.h"
Expand Down Expand Up @@ -400,4 +403,9 @@ class QgsOgrUtils
/** Quote a value for placement in a SQL string.
*/
static QString quotedValue( const QVariant& value );

static OGRDataSourceH OGROpenWrapper( const char* pszPath, bool bUpdate, OGRSFDriverH *phDriver );
static void OGRDestroyWrapper( OGRDataSourceH ogrDataSource );
};

#endif // QGSOGRPROVIDER_H
Loading

1 comment on commit f939e9c

@nirvn
Copy link
Contributor

@nirvn nirvn commented on f939e9c Jan 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rouault , this has created a side issue: when adding a geopackage layer (or opening a pre-existing project) simply to display its content, the modified date for the geopackage file is modified. This is somewhat problematic for users who actually use this file system attribute to keep track of when a dataset was actually last modified.

Is there a way to fix that side issue?

Please sign in to comment.