From d7f5bd0ea3776f34e8bfea2d7f7d358625cf9cc6 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Mon, 16 Sep 2019 15:12:19 +0200 Subject: [PATCH] Application options: defer disabling of drivers after application restart (relates to #29212) This is a follow-up of https://github.com/qgis/QGIS/pull/31772 Driver de-registration is now defered after application restart to avoid any risk of potential crashes if a dataset using a disabled driver was already in use. --- .../core/auto_generated/qgsapplication.sip.in | 26 ++++++++++++++ src/app/qgsoptions.cpp | 33 ++++++++++++++---- .../providers/gdal/qgsgdalproviderbase.cpp | 16 +++------ src/core/qgsapplication.cpp | 34 ++++++++++++++++++- src/core/qgsapplication.h | 34 +++++++++++++++++-- 5 files changed, 122 insertions(+), 21 deletions(-) diff --git a/python/core/auto_generated/qgsapplication.sip.in b/python/core/auto_generated/qgsapplication.sip.in index 6bba437d23a..0db3bd6bebf 100644 --- a/python/core/auto_generated/qgsapplication.sip.in +++ b/python/core/auto_generated/qgsapplication.sip.in @@ -585,6 +585,32 @@ Apply the skipped drivers list to gdal .. seealso:: :py:func:`restoreGdalDriver` .. seealso:: :py:func:`skippedGdalDrivers` +%End + + static void registerGdalDriversFromSettings(); +%Docstring +Register gdal drivers, excluding the ones mentioned in "gdal/skipList" setting. + +.. versionadded:: 3.10 +%End + + static QStringList deferredSkippedGdalDrivers(); +%Docstring +Returns the list of gdal drivers that have been disabled in the current session, +and thus, for safety, should not be disabled right now, but at the +next application restart. + +.. versionadded:: 3.10 +%End + + static void setSkippedGdalDrivers( const QStringList &skippedGdalDrivers, + const QStringList &deferredSkippedGdalDrivers ); +%Docstring +Sets the list of gdal drivers that should be disabled (``skippedGdalDrivers``), +but excludes for now the ones defines in ``deferredSkippedGdalDrivers``. +This writes the "gdal/skipList" setting. + +.. versionadded:: 3.10 %End static int maxThreads(); diff --git a/src/app/qgsoptions.cpp b/src/app/qgsoptions.cpp index ae969aa6a59..fa39023a967 100644 --- a/src/app/qgsoptions.cpp +++ b/src/app/qgsoptions.cpp @@ -2187,7 +2187,9 @@ void QgsOptions::optionsStackedWidget_CurrentChanged( int index ) void QgsOptions::loadGdalDriverList() { - QStringList mySkippedDrivers = QgsApplication::skippedGdalDrivers(); + QgsApplication::registerGdalDriversFromSettings(); + + const QStringList mySkippedDrivers = QgsApplication::skippedGdalDrivers(); GDALDriverH myGdalDriver; // current driver QString myGdalDriverDescription; QStringList myDrivers; @@ -2249,8 +2251,8 @@ void QgsOptions::loadGdalDriverList() myDriversLongName[myGdalDriverDescription] = QString( GDALGetMetadataItem( myGdalDriver, "DMD_LONGNAME", "" ) ); } - // restore GDAL_SKIP just in case - CPLSetConfigOption( "GDAL_SKIP", mySkippedDrivers.join( QStringLiteral( " " ) ).toUtf8() ); + // restore active drivers + QgsApplication::applyGdalSkippedDrivers(); myDrivers.removeDuplicates(); // myDrivers.sort(); @@ -2304,19 +2306,38 @@ void QgsOptions::loadGdalDriverList() void QgsOptions::saveGdalDriverList() { + bool driverUnregisterNeeded = false; + const auto oldSkippedGdalDrivers = QgsApplication::skippedGdalDrivers(); + auto deferredSkippedGdalDrivers = QgsApplication::deferredSkippedGdalDrivers(); + QStringList skippedGdalDrivers; for ( int i = 0; i < lstGdalDrivers->topLevelItemCount(); i++ ) { QTreeWidgetItem *mypItem = lstGdalDrivers->topLevelItem( i ); + const auto &driverName( mypItem->text( 0 ) ); if ( mypItem->checkState( 0 ) == Qt::Unchecked ) { - QgsApplication::skipGdalDriver( mypItem->text( 0 ) ); + skippedGdalDrivers << driverName; + if ( !deferredSkippedGdalDrivers.contains( driverName ) && + !oldSkippedGdalDrivers.contains( driverName ) ) + { + deferredSkippedGdalDrivers << driverName; + driverUnregisterNeeded = true; + } } else { - QgsApplication::restoreGdalDriver( mypItem->text( 0 ) ); + if ( deferredSkippedGdalDrivers.contains( driverName ) ) + { + deferredSkippedGdalDrivers.removeAll( driverName ); + } } } - mSettings->setValue( QStringLiteral( "gdal/skipList" ), QgsApplication::skippedGdalDrivers().join( QStringLiteral( " " ) ) ); + if ( driverUnregisterNeeded ) + { + QMessageBox::information( this, tr( "Drivers Disabled" ), + tr( "One or more drivers have been disabled. This will only take effect after QGIS is restarted." ) ); + } + QgsApplication::setSkippedGdalDrivers( skippedGdalDrivers, deferredSkippedGdalDrivers ); } void QgsOptions::addScale() diff --git a/src/core/providers/gdal/qgsgdalproviderbase.cpp b/src/core/providers/gdal/qgsgdalproviderbase.cpp index bd9fe838806..6ca71bedaca 100644 --- a/src/core/providers/gdal/qgsgdalproviderbase.cpp +++ b/src/core/providers/gdal/qgsgdalproviderbase.cpp @@ -26,6 +26,8 @@ #include "qgsgdalproviderbase.h" #include "qgssettings.h" +#include + QgsGdalProviderBase::QgsGdalProviderBase() { @@ -219,18 +221,8 @@ int QgsGdalProviderBase::colorInterpretationFromGdal( const GDALColorInterp gdal void QgsGdalProviderBase::registerGdalDrivers() { - GDALAllRegister(); - QgsSettings mySettings; - QString myJoinedList = mySettings.value( QStringLiteral( "gdal/skipList" ), "" ).toString(); - if ( !myJoinedList.isEmpty() ) - { - QStringList myList = myJoinedList.split( ' ' ); - for ( int i = 0; i < myList.size(); ++i ) - { - QgsApplication::skipGdalDriver( myList.at( i ) ); - } - QgsApplication::applyGdalSkippedDrivers(); - } + static std::once_flag initialized; + std::call_once( initialized, QgsApplication::registerGdalDriversFromSettings ); } QgsRectangle QgsGdalProviderBase::extent( GDALDatasetH gdalDataset )const diff --git a/src/core/qgsapplication.cpp b/src/core/qgsapplication.cpp index 09154d7a527..9f01d4a3c0d 100644 --- a/src/core/qgsapplication.cpp +++ b/src/core/qgsapplication.cpp @@ -128,6 +128,7 @@ QString ABISYM( QgsApplication::mCfgIntDir ); #endif QString ABISYM( QgsApplication::mBuildOutputPath ); QStringList ABISYM( QgsApplication::mGdalSkipList ); +QStringList QgsApplication::sDeferredSkippedGdalDrivers; int ABISYM( QgsApplication::mMaxThreads ); QString ABISYM( QgsApplication::mAuthDbDirPath ); @@ -1555,10 +1556,41 @@ void QgsApplication::restoreGdalDriver( const QString &driver ) applyGdalSkippedDrivers(); } +void QgsApplication::setSkippedGdalDrivers( const QStringList &skippedGdalDrivers, + const QStringList &deferredSkippedGdalDrivers ) +{ + ABISYM( mGdalSkipList ) = skippedGdalDrivers; + sDeferredSkippedGdalDrivers = deferredSkippedGdalDrivers; + + QgsSettings settings; + settings.setValue( QStringLiteral( "gdal/skipList" ), skippedGdalDrivers.join( QStringLiteral( " " ) ) ); + + applyGdalSkippedDrivers(); +} + +void QgsApplication::registerGdalDriversFromSettings() +{ + QgsSettings settings; + QString joinedList = settings.value( QStringLiteral( "gdal/skipList" ), QString() ).toString(); + QStringList myList; + if ( !joinedList.isEmpty() ) + { + myList = joinedList.split( ' ' ); + } + ABISYM( mGdalSkipList ) = myList; + applyGdalSkippedDrivers(); +} + void QgsApplication::applyGdalSkippedDrivers() { ABISYM( mGdalSkipList ).removeDuplicates(); - QString myDriverList = ABISYM( mGdalSkipList ).join( QStringLiteral( " " ) ); + QStringList realDisabledDriverList; + for ( const auto &driverName : ABISYM( mGdalSkipList ) ) + { + if ( !sDeferredSkippedGdalDrivers.contains( driverName ) ) + realDisabledDriverList << driverName; + } + QString myDriverList = realDisabledDriverList.join( ' ' ); QgsDebugMsg( QStringLiteral( "Gdal Skipped driver list set to:" ) ); QgsDebugMsg( myDriverList ); CPLSetConfigOption( "GDAL_SKIP", myDriverList.toUtf8() ); diff --git a/src/core/qgsapplication.h b/src/core/qgsapplication.h index 4f35dd736f7..e03acd5b062 100644 --- a/src/core/qgsapplication.h +++ b/src/core/qgsapplication.h @@ -515,7 +515,7 @@ class CORE_EXPORT QgsApplication : public QApplication * Sets the GDAL_SKIP environment variable to exclude the specified driver * and then calls GDALDriverManager::AutoSkipDrivers() to unregister it. The * driver name should be the short format of the Gdal driver name e.g. GTIFF. - */ + */ static void restoreGdalDriver( const QString &driver ); /** @@ -528,9 +528,33 @@ class CORE_EXPORT QgsApplication : public QApplication * Apply the skipped drivers list to gdal * \see skipGdalDriver * \see restoreGdalDriver - * \see skippedGdalDrivers */ + * \see skippedGdalDrivers + */ static void applyGdalSkippedDrivers(); + /** + * Register gdal drivers, excluding the ones mentioned in "gdal/skipList" setting. + * \since QGIS 3.10 + */ + static void registerGdalDriversFromSettings(); + + /** + * Returns the list of gdal drivers that have been disabled in the current session, + * and thus, for safety, should not be disabled right now, but at the + * next application restart. + * \since QGIS 3.10 + */ + static QStringList deferredSkippedGdalDrivers() { return sDeferredSkippedGdalDrivers; } + + /** + * Sets the list of gdal drivers that should be disabled (\a skippedGdalDrivers), + * but excludes for now the ones defines in \a deferredSkippedGdalDrivers. + * This writes the "gdal/skipList" setting. + * \since QGIS 3.10 + */ + static void setSkippedGdalDrivers( const QStringList &skippedGdalDrivers, + const QStringList &deferredSkippedGdalDrivers ); + /** * Gets maximum concurrent thread count * \since QGIS 2.4 */ @@ -858,6 +882,12 @@ class CORE_EXPORT QgsApplication : public QApplication * \see skipGdalDriver, restoreGdalDriver */ static QStringList ABISYM( mGdalSkipList ); + /** + * List of gdal drivers that have been disabled in the current session, + * and thus, for safety, should not be disabled right now, but at the + * next application restart */ + static QStringList sDeferredSkippedGdalDrivers; + /** * \since QGIS 2.4 */ static int ABISYM( mMaxThreads );