From 4515651b50cdd6e3e6f391bf5dfef8cd8c4cbb18 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Tue, 27 Aug 2024 15:18:14 +0200 Subject: [PATCH 1/9] Make GDALRasterBand::InterpolateAtPoint() method virtual --- gcore/gdal_priv.h | 8 ++++---- gcore/gdal_proxy.h | 5 +++++ gcore/gdalproxydataset.cpp | 6 ++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/gcore/gdal_priv.h b/gcore/gdal_priv.h index da9d7e5d5270..3b056f10cffd 100644 --- a/gcore/gdal_priv.h +++ b/gcore/gdal_priv.h @@ -1857,10 +1857,10 @@ class CPL_DLL GDALRasterBand : public GDALMajorObject std::shared_ptr AsMDArray() const; - CPLErr InterpolateAtPoint(double dfPixel, double dfLine, - GDALRIOResampleAlg eInterpolation, - double *pdfRealValue, - double *pdfImagValue = nullptr) const; + virtual CPLErr InterpolateAtPoint(double dfPixel, double dfLine, + GDALRIOResampleAlg eInterpolation, + double *pdfRealValue, + double *pdfImagValue = nullptr) const; #ifndef DOXYGEN_XML void ReportError(CPLErr eErrClass, CPLErrorNum err_no, const char *fmt, ...) diff --git a/gcore/gdal_proxy.h b/gcore/gdal_proxy.h index 518deb4b2d32..35da8d5e23bd 100644 --- a/gcore/gdal_proxy.h +++ b/gcore/gdal_proxy.h @@ -206,6 +206,11 @@ class CPL_DLL GDALProxyRasterBand : public GDALRasterBand GIntBig *pnLineSpace, char **papszOptions) override; + CPLErr InterpolateAtPoint(double dfPixel, double dfLine, + GDALRIOResampleAlg eInterpolation, + double *pdfRealValue, + double *pdfImagValue) const override; + private: CPL_DISALLOW_COPY_ASSIGN(GDALProxyRasterBand) }; diff --git a/gcore/gdalproxydataset.cpp b/gcore/gdalproxydataset.cpp index cb6fb251653d..e13b77ebf608 100644 --- a/gcore/gdalproxydataset.cpp +++ b/gcore/gdalproxydataset.cpp @@ -512,6 +512,12 @@ RB_PROXY_METHOD_WITH_RET(CPLVirtualMem *, nullptr, GetVirtualMemAuto, GIntBig *pnLineSpace, char **papszOptions), (eRWFlag, pnPixelSpace, pnLineSpace, papszOptions)) +RB_PROXY_METHOD_WITH_RET( + CPLErr, CE_Failure, InterpolateAtPoint, + (double dfPixel, double dfLine, GDALRIOResampleAlg eInterpolation, + double *pdfRealValue, double *pdfImagValue = nullptr) const, + (dfPixel, dfLine, eInterpolation, pdfRealValue, pdfImagValue)) + /************************************************************************/ /* UnrefUnderlyingRasterBand() */ /************************************************************************/ From 86d852604fe573f3b58f4f36cf05d5e2237b3f0f Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Wed, 28 Aug 2024 12:43:08 +0200 Subject: [PATCH 2/9] Make GDALRasterBand::EnablePixelTypeSignedByteWarning() method virtual --- gcore/gdal_priv.h | 2 +- gcore/gdal_proxy.h | 2 ++ gcore/gdalproxydataset.cpp | 10 ++++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/gcore/gdal_priv.h b/gcore/gdal_priv.h index 3b056f10cffd..7f9c6f8f04f0 100644 --- a/gcore/gdal_priv.h +++ b/gcore/gdal_priv.h @@ -1886,7 +1886,7 @@ class CPL_DLL GDALRasterBand : public GDALMajorObject //! @cond Doxygen_Suppress // Remove me in GDAL 4.0. See GetMetadataItem() implementation // Internal use in GDAL only ! - void EnablePixelTypeSignedByteWarning(bool b) + virtual void EnablePixelTypeSignedByteWarning(bool b) #ifndef GDAL_COMPILATION CPL_WARN_DEPRECATED("Do not use that method outside of GDAL!") #endif diff --git a/gcore/gdal_proxy.h b/gcore/gdal_proxy.h index 35da8d5e23bd..56b85c77add3 100644 --- a/gcore/gdal_proxy.h +++ b/gcore/gdal_proxy.h @@ -211,6 +211,8 @@ class CPL_DLL GDALProxyRasterBand : public GDALRasterBand double *pdfRealValue, double *pdfImagValue) const override; + void EnablePixelTypeSignedByteWarning(bool b) override; + private: CPL_DISALLOW_COPY_ASSIGN(GDALProxyRasterBand) }; diff --git a/gcore/gdalproxydataset.cpp b/gcore/gdalproxydataset.cpp index e13b77ebf608..51bf9bdb8e12 100644 --- a/gcore/gdalproxydataset.cpp +++ b/gcore/gdalproxydataset.cpp @@ -518,6 +518,16 @@ RB_PROXY_METHOD_WITH_RET( double *pdfRealValue, double *pdfImagValue = nullptr) const, (dfPixel, dfLine, eInterpolation, pdfRealValue, pdfImagValue)) +void GDALProxyRasterBand::EnablePixelTypeSignedByteWarning(bool b) +{ + GDALRasterBand *poSrcBand = RefUnderlyingRasterBand(); + if (poSrcBand) + { + poSrcBand->EnablePixelTypeSignedByteWarning(b); + UnrefUnderlyingRasterBand(poSrcBand); + } +} + /************************************************************************/ /* UnrefUnderlyingRasterBand() */ /************************************************************************/ From 027e957b6929e9a69ba49a6dcce6d56ff52cbd18 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Wed, 28 Aug 2024 13:08:17 +0200 Subject: [PATCH 3/9] Make GDALRasterBand::GetLockedBlockRef/TryGetLockedBlockRef/FlushBlock virtual; make GDALDataset::BlockBasedRasterIO virtual --- gcore/gdal_priv.h | 19 +++++++++++++------ gcore/gdal_proxy.h | 17 +++++++++++++++++ gcore/gdaldataset.cpp | 4 +--- gcore/gdalproxydataset.cpp | 36 +++++++++++++++++++++++++----------- gcore/rasterio.cpp | 2 +- 5 files changed, 57 insertions(+), 21 deletions(-) diff --git a/gcore/gdal_priv.h b/gcore/gdal_priv.h index 7f9c6f8f04f0..99453e75402d 100644 --- a/gcore/gdal_priv.h +++ b/gcore/gdal_priv.h @@ -579,7 +579,8 @@ class CPL_DLL GDALDataset : public GDALMajorObject GSpacing nLineSpace, GSpacing nBandSpace, GDALRasterIOExtraArg *psExtraArg) CPL_WARN_UNUSED_RESULT; - CPLErr + /* This method should only be be overloaded by GDALProxyDataset */ + virtual CPLErr BlockBasedRasterIO(GDALRWFlag eRWFlag, int nXOff, int nYOff, int nXSize, int nYSize, void *pData, int nBufXSize, int nBufYSize, GDALDataType eBufType, int nBandCount, @@ -1750,13 +1751,19 @@ class CPL_DLL GDALRasterBand : public GDALMajorObject CPLErr WriteBlock(int nXBlockOff, int nYBlockOff, void *pImage) CPL_WARN_UNUSED_RESULT; - GDALRasterBlock * + // This method should only be overloaded by GDALProxyRasterBand + virtual GDALRasterBlock * GetLockedBlockRef(int nXBlockOff, int nYBlockOff, int bJustInitialize = FALSE) CPL_WARN_UNUSED_RESULT; - GDALRasterBlock *TryGetLockedBlockRef(int nXBlockOff, int nYBlockYOff) - CPL_WARN_UNUSED_RESULT; - CPLErr FlushBlock(int nXBlockOff, int nYBlockOff, - int bWriteDirtyBlock = TRUE); + + // This method should only be overloaded by GDALProxyRasterBand + virtual GDALRasterBlock * + TryGetLockedBlockRef(int nXBlockOff, + int nYBlockYOff) CPL_WARN_UNUSED_RESULT; + + // This method should only be overloaded by GDALProxyRasterBand + virtual CPLErr FlushBlock(int nXBlockOff, int nYBlockOff, + int bWriteDirtyBlock = TRUE); unsigned char * GetIndexColorTranslationTo(/* const */ GDALRasterBand *poReferenceBand, diff --git a/gcore/gdal_proxy.h b/gcore/gdal_proxy.h index 56b85c77add3..76c85026c257 100644 --- a/gcore/gdal_proxy.h +++ b/gcore/gdal_proxy.h @@ -59,6 +59,13 @@ class CPL_DLL GDALProxyDataset : public GDALDataset CPLErr IRasterIO(GDALRWFlag, int, int, int, int, void *, int, int, GDALDataType, int, BANDMAP_TYPE, GSpacing, GSpacing, GSpacing, GDALRasterIOExtraArg *psExtraArg) override; + CPLErr BlockBasedRasterIO(GDALRWFlag eRWFlag, int nXOff, int nYOff, + int nXSize, int nYSize, void *pData, + int nBufXSize, int nBufYSize, + GDALDataType eBufType, int nBandCount, + const int *panBandMap, GSpacing nPixelSpace, + GSpacing nLineSpace, GSpacing nBandSpace, + GDALRasterIOExtraArg *psExtraArg) override; public: char **GetMetadataDomainList() override; @@ -140,6 +147,16 @@ class CPL_DLL GDALProxyRasterBand : public GDALRasterBand const char *pszDomain) override; CPLErr SetMetadataItem(const char *pszName, const char *pszValue, const char *pszDomain) override; + + GDALRasterBlock *GetLockedBlockRef(int nXBlockOff, int nYBlockOff, + int bJustInitialize) override; + + GDALRasterBlock *TryGetLockedBlockRef(int nXBlockOff, + int nYBlockYOff) override; + + CPLErr FlushBlock(int nXBlockOff, int nYBlockOff, + int bWriteDirtyBlock) override; + CPLErr FlushCache(bool bAtClosing) override; char **GetCategoryNames() override; double GetNoDataValue(int *pbSuccess = nullptr) override; diff --git a/gcore/gdaldataset.cpp b/gcore/gdaldataset.cpp index d52033c9e08c..f5a741ee18cd 100644 --- a/gcore/gdaldataset.cpp +++ b/gcore/gdaldataset.cpp @@ -748,9 +748,7 @@ CPLErr GDALDataset::BlockBasedFlushCache(bool bAtClosing) { for (int iBand = 0; iBand < nBands; ++iBand) { - GDALRasterBand *poBand = GetRasterBand(iBand + 1); - - const CPLErr eErr = poBand->FlushBlock(iX, iY); + const CPLErr eErr = papoBands[iBand]->FlushBlock(iX, iY); if (eErr != CE_None) return CE_Failure; diff --git a/gcore/gdalproxydataset.cpp b/gcore/gdalproxydataset.cpp index 51bf9bdb8e12..5cd560f0024b 100644 --- a/gcore/gdalproxydataset.cpp +++ b/gcore/gdalproxydataset.cpp @@ -137,6 +137,17 @@ CPLErr GDALProxyDataset::IRasterIO( return ret; } +D_PROXY_METHOD_WITH_RET(CPLErr, CE_Failure, BlockBasedRasterIO, + (GDALRWFlag eRWFlag, int nXOff, int nYOff, int nXSize, + int nYSize, void *pData, int nBufXSize, int nBufYSize, + GDALDataType eBufType, int nBandCount, + const int *panBandMap, GSpacing nPixelSpace, + GSpacing nLineSpace, GSpacing nBandSpace, + GDALRasterIOExtraArg *psExtraArg), + (eRWFlag, nXOff, nYOff, nXSize, nYSize, pData, + nBufXSize, nBufYSize, eBufType, nBandCount, panBandMap, + nPixelSpace, nLineSpace, nBandSpace, psExtraArg)) + D_PROXY_METHOD_WITH_RET(CPLErr, CE_Failure, IBuildOverviews, (const char *pszResampling, int nOverviews, const int *panOverviewList, int nListBands, @@ -159,17 +170,8 @@ D_PROXY_METHOD_WITH_RET(CPLErr, CE_Failure, ReadCompressedData, panBandList, ppBuffer, pnBufferSize, ppszDetailedFormat)) -CPLErr GDALProxyDataset::FlushCache(bool bAtClosing) -{ - CPLErr eErr = CE_None; - GDALDataset *poUnderlyingDataset = RefUnderlyingDataset(); - if (poUnderlyingDataset) - { - eErr = poUnderlyingDataset->FlushCache(bAtClosing); - UnrefUnderlyingDataset(poUnderlyingDataset); - } - return eErr; -} +D_PROXY_METHOD_WITH_RET(CPLErr, CE_None, FlushCache, (bool bAtClosing), + (bAtClosing)) D_PROXY_METHOD_WITH_RET(char **, nullptr, GetMetadataDomainList, (), ()) D_PROXY_METHOD_WITH_RET(char **, nullptr, GetMetadata, (const char *pszDomain), @@ -386,6 +388,18 @@ RB_PROXY_METHOD_WITH_RET(CPLErr, CE_Failure, SetMetadataItem, const char *pszDomain), (pszName, pszValue, pszDomain)) +RB_PROXY_METHOD_WITH_RET(GDALRasterBlock *, nullptr, GetLockedBlockRef, + (int nXBlockOff, int nYBlockOff, int bJustInitialize), + (nXBlockOff, nYBlockOff, bJustInitialize)) + +RB_PROXY_METHOD_WITH_RET(GDALRasterBlock *, nullptr, TryGetLockedBlockRef, + (int nXBlockOff, int nYBlockOff), + (nXBlockOff, nYBlockOff)) + +RB_PROXY_METHOD_WITH_RET(CPLErr, CE_Failure, FlushBlock, + (int nXBlockOff, int nYBlockOff, int bWriteDirtyBlock), + (nXBlockOff, nYBlockOff, bWriteDirtyBlock)) + CPLErr GDALProxyRasterBand::FlushCache(bool bAtClosing) { // We need to make sure that all cached bocks at the proxy level are diff --git a/gcore/rasterio.cpp b/gcore/rasterio.cpp index 04be0bc5968b..0f15a3168a69 100644 --- a/gcore/rasterio.cpp +++ b/gcore/rasterio.cpp @@ -4089,7 +4089,7 @@ CPLErr GDALDataset::BlockBasedRasterIO( { GDALRasterBand *poBand = GetRasterBand(panBandMap[iBand]); - eErr = poBand->GDALRasterBand::IRasterIO( + eErr = poBand->IRasterIO( eRWFlag, nChunkXOff, nChunkYOff, nChunkXSize, nChunkYSize, pabyChunkData + From 70b01431e62550d16cef03e75277206dac084386 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Wed, 28 Aug 2024 21:45:55 +0200 Subject: [PATCH 4/9] GDALRasterBand: tag more methods as const --- gcore/gdal_priv.h | 28 +++++++++++++++++++--------- gcore/gdalrasterband.cpp | 16 ++++++++-------- gcore/rasterio.cpp | 1 + 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/gcore/gdal_priv.h b/gcore/gdal_priv.h index 99453e75402d..4ee9f5822a6c 100644 --- a/gcore/gdal_priv.h +++ b/gcore/gdal_priv.h @@ -1583,6 +1583,11 @@ class CPL_DLL GDALRasterBand : public GDALMajorObject m_poBandRef = bOwned ? nullptr : poBand; } + const GDALRasterBand *get() const + { + return static_cast(*this); + } + GDALRasterBand *get() { return static_cast(*this); @@ -1593,6 +1598,11 @@ class CPL_DLL GDALRasterBand : public GDALMajorObject return m_poBandOwned != nullptr; } + operator const GDALRasterBand *() const + { + return m_poBandOwned ? m_poBandOwned.get() : m_poBandRef; + } + operator GDALRasterBand *() { return m_poBandOwned ? m_poBandOwned.get() : m_poBandRef; @@ -1680,15 +1690,15 @@ class CPL_DLL GDALRasterBand : public GDALMajorObject ~GDALRasterBand() override; - int GetXSize(); - int GetYSize(); - int GetBand(); - GDALDataset *GetDataset(); + int GetXSize() const; + int GetYSize() const; + int GetBand() const; + GDALDataset *GetDataset() const; - GDALDataType GetRasterDataType(void); - void GetBlockSize(int *pnXSize, int *pnYSize); + GDALDataType GetRasterDataType(void) const; + void GetBlockSize(int *pnXSize, int *pnYSize) const; CPLErr GetActualBlockSize(int nXBlockOff, int nYBlockOff, int *pnXValid, - int *pnYValid); + int *pnYValid) const; virtual GDALSuggestedBlockAccessPattern GetSuggestedBlockAccessPattern() const; @@ -1870,8 +1880,8 @@ class CPL_DLL GDALRasterBand : public GDALMajorObject double *pdfImagValue = nullptr) const; #ifndef DOXYGEN_XML - void ReportError(CPLErr eErrClass, CPLErrorNum err_no, const char *fmt, ...) - CPL_PRINT_FUNC_FORMAT(4, 5); + void ReportError(CPLErr eErrClass, CPLErrorNum err_no, const char *fmt, + ...) const CPL_PRINT_FUNC_FORMAT(4, 5); #endif /** Convert a GDALRasterBand* to a GDALRasterBandH. diff --git a/gcore/gdalrasterband.cpp b/gcore/gdalrasterband.cpp index 05b16c89a729..34914188e5a8 100644 --- a/gcore/gdalrasterband.cpp +++ b/gcore/gdalrasterband.cpp @@ -1332,7 +1332,7 @@ CPLErr CPL_STDCALL GDALWriteBlock(GDALRasterBandH hBand, int nXOff, int nYOff, * @since GDAL 2.2 */ CPLErr GDALRasterBand::GetActualBlockSize(int nXBlockOff, int nYBlockOff, - int *pnXValid, int *pnYValid) + int *pnXValid, int *pnYValid) const { if (nXBlockOff < 0 || nBlockXSize == 0 || nXBlockOff >= DIV_ROUND_UP(nRasterXSize, nBlockXSize) || @@ -1434,7 +1434,7 @@ GDALRasterBand::GetSuggestedBlockAccessPattern() const * @return the data type of pixels for this band. */ -GDALDataType GDALRasterBand::GetRasterDataType() +GDALDataType GDALRasterBand::GetRasterDataType() const { return eDataType; @@ -1485,7 +1485,7 @@ GDALDataType CPL_STDCALL GDALGetRasterDataType(GDALRasterBandH hBand) * @param pnYSize integer to put the Y block size into or NULL. */ -void GDALRasterBand::GetBlockSize(int *pnXSize, int *pnYSize) +void GDALRasterBand::GetBlockSize(int *pnXSize, int *pnYSize) const { if (nBlockXSize <= 0 || nBlockYSize <= 0) @@ -3732,7 +3732,7 @@ CPLErr CPL_STDCALL GDALSetRasterUnitType(GDALRasterBandH hBand, * @return the width in pixels of this band. */ -int GDALRasterBand::GetXSize() +int GDALRasterBand::GetXSize() const { return nRasterXSize; @@ -3769,7 +3769,7 @@ int CPL_STDCALL GDALGetRasterBandXSize(GDALRasterBandH hBand) * @return the height in pixels of this band. */ -int GDALRasterBand::GetYSize() +int GDALRasterBand::GetYSize() const { return nRasterYSize; @@ -3811,7 +3811,7 @@ int CPL_STDCALL GDALGetRasterBandYSize(GDALRasterBandH hBand) * @return band number (1+) or 0 if the band number isn't known. */ -int GDALRasterBand::GetBand() +int GDALRasterBand::GetBand() const { return nBand; @@ -3852,7 +3852,7 @@ int CPL_STDCALL GDALGetBandNumber(GDALRasterBandH hBand) * NULL if this cannot be determined. */ -GDALDataset *GDALRasterBand::GetDataset() +GDALDataset *GDALRasterBand::GetDataset() const { return poDS; @@ -8437,7 +8437,7 @@ void GDALRasterBand::IncDirtyBlocks(int nInc) */ void GDALRasterBand::ReportError(CPLErr eErrClass, CPLErrorNum err_no, - const char *fmt, ...) + const char *fmt, ...) const { va_list args; diff --git a/gcore/rasterio.cpp b/gcore/rasterio.cpp index 0f15a3168a69..8f6a99ee48eb 100644 --- a/gcore/rasterio.cpp +++ b/gcore/rasterio.cpp @@ -454,6 +454,7 @@ CPLErr GDALRasterBand::IRasterIO(GDALRWFlag eRWFlag, int nXOff, int nYOff, nXOff <= nLBlockX * nBlockXSize && nYOff <= nLBlockY * nBlockYSize && (nXOff + nXSize >= nXRight || + // cppcheck-suppress knownConditionTrueFalse (nXOff + nXSize == GetXSize() && nXRight > GetXSize())) && (nYOff + nYSize - nBlockYSize >= nLBlockY * nBlockYSize || (nYOff + nYSize == GetYSize() && From 7ac2d7df483cd04f299d67132c8fdd8fcc4f2e41 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Thu, 29 Aug 2024 15:08:09 +0200 Subject: [PATCH 5/9] Python bindings: Dataset.Close(): invalidate children before closing dataset --- swig/include/python/gdal_python.i | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/swig/include/python/gdal_python.i b/swig/include/python/gdal_python.i index 0f6cb1bfc347..f6fc9feeea18 100644 --- a/swig/include/python/gdal_python.i +++ b/swig/include/python/gdal_python.i @@ -1614,10 +1614,13 @@ CPLErr ReadRaster1( double xoff, double yoff, double xsize, double ysize, return get(value) %} +%feature("pythonprepend") Close %{ + self._invalidate_children() +%} + %feature("pythonappend") Close %{ self.thisown = 0 self.this = None - self._invalidate_children() %} %feature("shadow") ExecuteSQL %{ From 8823ea42fae4853f31540251835c4e10521930e3 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Wed, 28 Aug 2024 12:44:01 +0200 Subject: [PATCH 6/9] [RFC 101] Add GDALGetThreadSafeDataset() and GDAL_OF_THREAD_SAFE --- apps/CMakeLists.txt | 3 + apps/multireadtest.cpp | 139 +- autotest/gcore/thread_test.py | 351 +++++ doc/source/spelling_wordlist.txt | 1 + doc/source/user/multithreading.rst | 24 +- frmts/mem/memdataset.cpp | 135 +- frmts/mem/memdataset.h | 15 +- gcore/CMakeLists.txt | 1 + gcore/gdal.h | 13 + gcore/gdal_pam.h | 2 + gcore/gdal_priv.h | 22 +- gcore/gdaldataset.cpp | 161 ++- gcore/gdaldriver.cpp | 8 +- gcore/gdalpamrasterband.cpp | 55 + gcore/gdalthreadsafedataset.cpp | 1169 +++++++++++++++++ .../sqlite/ogrsqlitedatasource.cpp | 2 +- port/cpl_mem_cache.h | 4 + swig/include/Band.i | 4 + swig/include/Dataset.i | 11 + swig/include/gdalconst.i | 1 + swig/include/java/gdal_java.i | 3 +- swig/include/python/gdal_python.i | 10 + 22 files changed, 2057 insertions(+), 77 deletions(-) create mode 100644 gcore/gdalthreadsafedataset.cpp diff --git a/apps/CMakeLists.txt b/apps/CMakeLists.txt index 554ca13f1955..4ce116ae1b01 100644 --- a/apps/CMakeLists.txt +++ b/apps/CMakeLists.txt @@ -158,6 +158,9 @@ if (BUILD_APPS) add_executable(gdalasyncread EXCLUDE_FROM_ALL gdalasyncread.cpp) add_executable(gdalwarpsimple EXCLUDE_FROM_ALL gdalwarpsimple.c) add_executable(multireadtest EXCLUDE_FROM_ALL multireadtest.cpp) + if(NOT MSVC AND CMAKE_THREAD_LIBS_INIT) + target_link_libraries(multireadtest PRIVATE ${CMAKE_THREAD_LIBS_INIT}) + endif() add_executable(test_ogrsf test_ogrsf.cpp) add_executable(testreprojmulti EXCLUDE_FROM_ALL testreprojmulti.cpp) diff --git a/apps/multireadtest.cpp b/apps/multireadtest.cpp index d8a4e1cc24b8..e0c737bcf521 100644 --- a/apps/multireadtest.cpp +++ b/apps/multireadtest.cpp @@ -30,27 +30,32 @@ #include "gdal_alg.h" #include "cpl_multiproc.h" #include "cpl_string.h" + +#include +#include +#include #include static int nIterations = 1; static bool bLockOnOpen = false; static int nOpenIterations = 1; static volatile int nPendingThreads = 0; +static bool bThreadCanFinish = false; +static std::mutex oMutex; +static std::condition_variable oCond; static const char *pszFilename = nullptr; static int nChecksum = 0; static int nWidth = 0; static int nHeight = 0; -static CPLMutex *pGlobalMutex = nullptr; - /************************************************************************/ /* Usage() */ /************************************************************************/ static void Usage() { - printf("multireadtest [-lock_on_open] [-open_in_main] [-t ]\n" - " [-i ] [-oi ]\n" + printf("multireadtest [[-thread_safe] | [[-lock_on_open] [-open_in_main]]\n" + " [-t ] [-i ] [-oi ]\n" " [-width ] [-height ]\n" " filename\n"); exit(1); @@ -74,12 +79,12 @@ static void WorkerFunc(void *arg) else { if (bLockOnOpen) - CPLAcquireMutex(pGlobalMutex, 100.0); + oMutex.lock(); hDS = GDALOpen(pszFilename, GA_ReadOnly); if (bLockOnOpen) - CPLReleaseMutex(pGlobalMutex); + oMutex.unlock(); } for (int iIter = 0; iIter < nIterations && hDS != nullptr; iIter++) @@ -99,10 +104,10 @@ static void WorkerFunc(void *arg) if (hDS && hDSIn == nullptr) { if (bLockOnOpen) - CPLAcquireMutex(pGlobalMutex, 100.0); + oMutex.lock(); GDALClose(hDS); if (bLockOnOpen) - CPLReleaseMutex(pGlobalMutex); + oMutex.unlock(); } else if (hDSIn != nullptr) { @@ -110,9 +115,13 @@ static void WorkerFunc(void *arg) } } - CPLAcquireMutex(pGlobalMutex, 100.0); - nPendingThreads--; - CPLReleaseMutex(pGlobalMutex); + { + std::unique_lock oLock(oMutex); + nPendingThreads--; + oCond.notify_all(); + while (!bThreadCanFinish) + oCond.wait(oLock); + } } /************************************************************************/ @@ -131,6 +140,10 @@ int main(int argc, char **argv) int nThreadCount = 4; bool bOpenInThreads = true; + bool bThreadSafe = false; + bool bJoinAfterClosing = false; + bool bDetach = false; + bool bClose = true; for (int iArg = 1; iArg < argc; iArg++) { @@ -154,6 +167,10 @@ int main(int argc, char **argv) { nHeight = atoi(argv[++iArg]); } + else if (EQUAL(argv[iArg], "-thread_safe")) + { + bThreadSafe = true; + } else if (EQUAL(argv[iArg], "-lock_on_open")) { bLockOnOpen = true; @@ -162,6 +179,18 @@ int main(int argc, char **argv) { bOpenInThreads = false; } + else if (EQUAL(argv[iArg], "-join_after_closing")) + { + bJoinAfterClosing = true; + } + else if (EQUAL(argv[iArg], "-detach")) + { + bDetach = true; + } + else if (EQUAL(argv[iArg], "-do_not_close")) + { + bClose = false; + } else if (pszFilename == nullptr) { pszFilename = argv[iArg]; @@ -186,12 +215,10 @@ int main(int argc, char **argv) /* -------------------------------------------------------------------- */ /* Get the checksum of band1. */ /* -------------------------------------------------------------------- */ - GDALDatasetH hDS = nullptr; - GDALAllRegister(); for (int i = 0; i < 2; i++) { - hDS = GDALOpen(pszFilename, GA_ReadOnly); + GDALDatasetH hDS = GDALOpen(pszFilename, GA_ReadOnly); if (hDS == nullptr) exit(1); @@ -210,39 +237,83 @@ int main(int argc, char **argv) /* -------------------------------------------------------------------- */ /* Fire off worker threads. */ /* -------------------------------------------------------------------- */ - pGlobalMutex = CPLCreateMutex(); - CPLReleaseMutex(pGlobalMutex); nPendingThreads = nThreadCount; + GDALDatasetH hThreadSafeDS = nullptr; + if (bThreadSafe) + { + hThreadSafeDS = + GDALOpenEx(pszFilename, GDAL_OF_RASTER | GDAL_OF_THREAD_SAFE, + nullptr, nullptr, nullptr); + if (!hThreadSafeDS) + exit(1); + } + std::vector aoThreads; std::vector aoDS; for (int iThread = 0; iThread < nThreadCount; iThread++) { - hDS = nullptr; - if (!bOpenInThreads) + GDALDatasetH hDS = nullptr; + if (bThreadSafe) { - hDS = GDALOpen(pszFilename, GA_ReadOnly); - if (!hDS) + hDS = hThreadSafeDS; + } + else + { + if (!bOpenInThreads) { - printf("GDALOpen() failed.\n"); - exit(1); + hDS = GDALOpen(pszFilename, GA_ReadOnly); + if (!hDS) + { + printf("GDALOpen() failed.\n"); + exit(1); + } + aoDS.push_back(hDS); } - aoDS.push_back(hDS); } - if (CPLCreateThread(WorkerFunc, hDS) == -1) + aoThreads.push_back(std::thread([hDS]() { WorkerFunc(hDS); })); + } + + { + std::unique_lock oLock(oMutex); + while (nPendingThreads > 0) { - printf("CPLCreateThread() failed.\n"); - exit(1); + // printf("nPendingThreads = %d\n", nPendingThreads); + oCond.wait(oLock); } } - while (nPendingThreads > 0) - CPLSleep(0.5); - - CPLDestroyMutex(pGlobalMutex); + if (!bJoinAfterClosing && !bDetach) + { + { + std::lock_guard oLock(oMutex); + bThreadCanFinish = true; + oCond.notify_all(); + } + for (auto &oThread : aoThreads) + oThread.join(); + } for (size_t i = 0; i < aoDS.size(); ++i) GDALClose(aoDS[i]); + if (bClose) + GDALClose(hThreadSafeDS); + + if (bDetach) + { + for (auto &oThread : aoThreads) + oThread.detach(); + } + else if (bJoinAfterClosing) + { + { + std::lock_guard oLock(oMutex); + bThreadCanFinish = true; + oCond.notify_all(); + } + for (auto &oThread : aoThreads) + oThread.join(); + } printf("All threads complete.\n"); @@ -250,5 +321,13 @@ int main(int argc, char **argv) GDALDestroyDriverManager(); + { + std::lock_guard oLock(oMutex); + bThreadCanFinish = true; + oCond.notify_all(); + } + + printf("End of main.\n"); + return 0; } diff --git a/autotest/gcore/thread_test.py b/autotest/gcore/thread_test.py index 9fe563757aae..f57281c00fc2 100755 --- a/autotest/gcore/thread_test.py +++ b/autotest/gcore/thread_test.py @@ -29,6 +29,7 @@ # DEALINGS IN THE SOFTWARE. ############################################################################### +import shutil import threading import gdaltest @@ -75,3 +76,353 @@ def test_thread_test_1(): ret = False assert ret + + +def launch_threads(get_band, expected_cs, on_mask_band=False): + res = [True] + + def verify_checksum(): + for i in range(1000): + got_cs = get_band().Checksum() + if got_cs != expected_cs: + res[0] = False + assert False, (got_cs, expected_cs) + + threads = [threading.Thread(target=verify_checksum)] + for t in threads: + t.start() + for t in threads: + t.join() + assert res[0] + + +def test_thread_safe_open(): + + ds = gdal.OpenEx("data/byte.tif", gdal.OF_RASTER | gdal.OF_THREAD_SAFE) + assert ds.IsThreadSafe(gdal.OF_RASTER) + assert not ds.IsThreadSafe(gdal.OF_RASTER | gdal.OF_UPDATE) + + def get_band(): + return ds.GetRasterBand(1) + + launch_threads(get_band, 4672) + + # Check that GetThreadSafeDataset() on an already thread-safe dataset + # return itself. + ref_count_before = ds.GetRefCount() + thread_safe_ds = ds.GetThreadSafeDataset(gdal.OF_RASTER) + assert ds.GetRefCount() == ref_count_before + 1 + assert thread_safe_ds.GetRefCount() == ref_count_before + 1 + del thread_safe_ds + assert ds.GetRefCount() == ref_count_before + + +def test_thread_safe_create(): + + ds = gdal.OpenEx("data/byte.tif", gdal.OF_RASTER) + assert not ds.IsThreadSafe(gdal.OF_RASTER) + assert ds.GetRefCount() == 1 + thread_safe_ds = ds.GetThreadSafeDataset(gdal.OF_RASTER) + assert thread_safe_ds.IsThreadSafe(gdal.OF_RASTER) + assert ds.GetRefCount() == 2 + del ds + + def get_band(): + return thread_safe_ds.GetRasterBand(1) + + launch_threads(get_band, 4672) + + +def test_thread_safe_create_close_src_ds(): + + ds = gdal.OpenEx("data/byte.tif", gdal.OF_RASTER) + thread_safe_ds = ds.GetThreadSafeDataset(gdal.OF_RASTER) + ds.Close() + with pytest.raises(Exception): + thread_safe_ds.RasterCount + + +def test_thread_safe_src_cannot_be_reopened(tmp_vsimem): + + tmpfilename = str(tmp_vsimem / "byte.tif") + gdal.Translate(tmpfilename, "data/byte.tif") + + with gdal.OpenEx(tmpfilename, gdal.OF_RASTER | gdal.OF_THREAD_SAFE) as ds: + gdal.Unlink(tmpfilename) + with pytest.raises(Exception): + ds.GetRasterBand(1).Checksum() + + +@pytest.mark.parametrize( + "flag", [gdal.OF_UPDATE, gdal.OF_VECTOR, gdal.OF_MULTIDIM_RASTER, gdal.OF_GNM] +) +def test_thread_safe_incompatible_open_flags(flag): + with pytest.raises(Exception, match="mutually exclusive"): + gdal.OpenEx("data/byte.tif", gdal.OF_THREAD_SAFE | flag) + + +def test_thread_safe_src_alter_after_opening(tmp_vsimem): + + tmpfilename = str(tmp_vsimem / "byte.tif") + + gdal.Translate(tmpfilename, "data/byte.tif") + with gdal.OpenEx(tmpfilename, gdal.OF_RASTER | gdal.OF_THREAD_SAFE) as ds: + gdal.GetDriverByName("GTiff").Create( + tmpfilename, ds.RasterXSize + 1, ds.RasterYSize, ds.RasterCount + ) + with pytest.raises(Exception): + ds.GetRasterBand(1).Checksum() + + gdal.Translate(tmpfilename, "data/byte.tif") + with gdal.OpenEx(tmpfilename, gdal.OF_RASTER | gdal.OF_THREAD_SAFE) as ds: + gdal.GetDriverByName("GTiff").Create( + tmpfilename, ds.RasterXSize, ds.RasterYSize + 1, ds.RasterCount + ) + with pytest.raises(Exception): + ds.GetRasterBand(1).Checksum() + + gdal.Translate(tmpfilename, "data/byte.tif") + with gdal.OpenEx(tmpfilename, gdal.OF_RASTER | gdal.OF_THREAD_SAFE) as ds: + gdal.GetDriverByName("GTiff").Create( + tmpfilename, ds.RasterXSize, ds.RasterYSize, ds.RasterCount + 1 + ) + with pytest.raises(Exception): + ds.GetRasterBand(1).Checksum() + + gdal.Translate(tmpfilename, "data/byte.tif") + with gdal.OpenEx(tmpfilename, gdal.OF_RASTER | gdal.OF_THREAD_SAFE) as ds: + gdal.GetDriverByName("GTiff").Create( + tmpfilename, ds.RasterXSize, ds.RasterYSize, ds.RasterCount, gdal.GDT_Int16 + ) + with pytest.raises(Exception): + ds.GetRasterBand(1).Checksum() + + gdal.Translate(tmpfilename, "data/byte.tif") + with gdal.OpenEx(tmpfilename, gdal.OF_RASTER | gdal.OF_THREAD_SAFE) as ds: + gdal.GetDriverByName("GTiff").Create( + tmpfilename, + ds.RasterXSize, + ds.RasterYSize, + ds.RasterCount, + options=["TILED=YES"], + ) + with pytest.raises(Exception): + ds.GetRasterBand(1).Checksum() + + with gdal.Translate(tmpfilename, "data/byte.tif") as ds: + ds.BuildOverviews("NEAR", [2]) + with gdal.OpenEx(tmpfilename, gdal.OF_RASTER | gdal.OF_THREAD_SAFE) as ds: + ds.GetRasterBand(1).GetOverviewCount() + gdal.GetDriverByName("GTiff").Create( + tmpfilename, ds.RasterXSize, ds.RasterYSize, ds.RasterCount + ) + with pytest.raises(Exception): + ds.GetRasterBand(1).GetOverview(0).Checksum() + + +def test_thread_safe_mask_band(): + + with gdal.Open("data/stefan_full_rgba.tif") as src_ds: + expected_cs = src_ds.GetRasterBand(1).GetMaskBand().Checksum() + + with gdal.OpenEx( + "data/stefan_full_rgba.tif", gdal.OF_RASTER | gdal.OF_THREAD_SAFE + ) as ds: + + def get_band(): + return ds.GetRasterBand(1).GetMaskBand() + + launch_threads(get_band, expected_cs) + + with gdal.OpenEx( + "data/stefan_full_rgba.tif", gdal.OF_RASTER | gdal.OF_THREAD_SAFE + ) as ds: + band = ds.GetRasterBand(1).GetMaskBand() + + def get_band(): + return band + + launch_threads(get_band, expected_cs) + + +def test_thread_safe_mask_of_mask_band(): + + with gdal.Open("data/stefan_full_rgba.tif") as src_ds: + expected_cs = src_ds.GetRasterBand(1).GetMaskBand().GetMaskBand().Checksum() + + with gdal.OpenEx( + "data/stefan_full_rgba.tif", gdal.OF_RASTER | gdal.OF_THREAD_SAFE + ) as ds: + + def get_band(): + return ds.GetRasterBand(1).GetMaskBand().GetMaskBand() + + launch_threads(get_band, expected_cs) + + +def test_thread_safe_mask_band_implicit_mem_ds(): + + with gdal.Open("data/stefan_full_rgba.tif") as src_ds: + ds = gdal.GetDriverByName("MEM").CreateCopy("", src_ds) + expected_cs = src_ds.GetRasterBand(1).GetMaskBand().Checksum() + assert ds.GetRasterBand(1).GetMaskBand().Checksum() == expected_cs + + ds = ds.GetThreadSafeDataset(gdal.OF_RASTER) + + def get_band(): + return ds.GetRasterBand(1).GetMaskBand() + + launch_threads(get_band, expected_cs) + + +def test_thread_safe_mask_band_explicit_mem_ds(): + + with gdal.Open("data/stefan_full_rgba.tif") as src_ds: + ds = gdal.GetDriverByName("MEM").Create( + "", src_ds.RasterXSize, src_ds.RasterYSize, 1 + ) + ds.GetRasterBand(1).CreateMaskBand(gdal.GMF_PER_DATASET) + ds.GetRasterBand(1).GetMaskBand().WriteRaster( + 0, + 0, + src_ds.RasterXSize, + src_ds.RasterYSize, + src_ds.GetRasterBand(1).GetMaskBand().ReadRaster(), + ) + + expected_cs = src_ds.GetRasterBand(1).GetMaskBand().Checksum() + assert ds.GetRasterBand(1).GetMaskBand().Checksum() == expected_cs + + ds = ds.GetThreadSafeDataset(gdal.OF_RASTER) + + def get_band(): + return ds.GetRasterBand(1).GetMaskBand() + + launch_threads(get_band, expected_cs) + + +def test_thread_safe_overview(tmp_path): + + tmpfilename = str(tmp_path / "byte.tif") + shutil.copy("data/byte.tif", tmpfilename) + with gdal.Open(tmpfilename, gdal.GA_Update) as ds: + ds.BuildOverviews("NEAR", [2]) + expected_cs = ds.GetRasterBand(1).GetOverview(0).Checksum() + + with gdal.OpenEx(tmpfilename, gdal.OF_RASTER | gdal.OF_THREAD_SAFE) as ds: + assert ds.GetRasterBand(1).GetOverviewCount() == 1 + assert ds.GetRasterBand(1).GetOverview(-1) is None + assert ds.GetRasterBand(1).GetOverview(1) is None + + def get_band(): + return ds.GetRasterBand(1).GetOverview(0) + + launch_threads(get_band, expected_cs) + + with gdal.OpenEx(tmpfilename, gdal.OF_RASTER | gdal.OF_THREAD_SAFE) as ds: + band = ds.GetRasterBand(1).GetOverview(0) + + def get_band(): + return band + + launch_threads(get_band, expected_cs) + + +def test_thread_safe_overview_mem_ds(): + + with gdal.Open("data/byte.tif") as src_ds: + ds = gdal.GetDriverByName("MEM").CreateCopy("", src_ds) + ds.BuildOverviews("NEAR", [2]) + expected_cs = ds.GetRasterBand(1).GetOverview(0).Checksum() + + ds = ds.GetThreadSafeDataset(gdal.OF_RASTER) + + assert ds.GetRasterBand(1).GetOverviewCount() == 1 + + def get_band(): + return ds.GetRasterBand(1).GetOverview(0) + + launch_threads(get_band, expected_cs) + + def get_band(): + return ds.GetRasterBand(1).GetSampleOverview(1) + + launch_threads(get_band, expected_cs) + + band = ds.GetRasterBand(1).GetOverview(0) + + def get_band(): + return band + + launch_threads(get_band, expected_cs) + + +def test_thread_safe_open_options(tmp_path): + + tmpfilename = str(tmp_path / "byte.tif") + shutil.copy("data/byte.tif", tmpfilename) + with gdal.Open(tmpfilename, gdal.GA_Update) as ds: + ds.BuildOverviews("NEAR", [2]) + expected_cs = ds.GetRasterBand(1).GetOverview(0).Checksum() + + with gdal.OpenEx( + tmpfilename, + gdal.OF_RASTER | gdal.OF_THREAD_SAFE, + open_options=["OVERVIEW_LEVEL=0"], + ) as ds: + + def get_band(): + return ds.GetRasterBand(1) + + launch_threads(get_band, expected_cs) + + +@pytest.mark.require_driver("HDF5") +@pytest.mark.require_driver("netCDF") +def test_thread_safe_reuse_same_driver_as_prototype(): + """Checks that thread-safe mode honours the opening driver""" + + with gdal.OpenEx( + "../gdrivers/data/netcdf/byte_hdf5_starting_at_offset_1024.nc", + gdal.OF_RASTER | gdal.OF_THREAD_SAFE, + allowed_drivers=["HDF5"], + ) as ds: + + ret = [False] + + def thread_func(): + assert ds.GetMetadataItem("_NCProperties") is not None + ret[0] = True + + t = threading.Thread(target=thread_func) + t.start() + t.join() + assert ret[0] + + +def test_thread_safe_no_rat(): + + ds = gdal.GetDriverByName("MEM").Create("", 1, 1) + ds = ds.GetThreadSafeDataset(gdal.OF_RASTER) + assert ds.GetRasterBand(1).GetDefaultRAT() is None + + +def test_thread_safe_rat(): + + ds = gdal.GetDriverByName("MEM").Create("", 1, 1) + ds.GetRasterBand(1).SetDefaultRAT(gdal.RasterAttributeTable()) + ds = ds.GetThreadSafeDataset(gdal.OF_RASTER) + assert ds.GetRasterBand(1).GetDefaultRAT() is not None + + +@pytest.mark.require_driver("HFA") +def test_thread_safe_unsupported_rat(): + + with gdal.OpenEx( + "../gdrivers/data/hfa/87test.img", gdal.OF_RASTER | gdal.OF_THREAD_SAFE + ) as ds: + with pytest.raises( + Exception, + match="not supporting a non-GDALDefaultRasterAttributeTable implementation", + ): + ds.GetRasterBand(1).GetDefaultRAT() diff --git a/doc/source/spelling_wordlist.txt b/doc/source/spelling_wordlist.txt index 8aec96480a48..33fc5b20aca7 100644 --- a/doc/source/spelling_wordlist.txt +++ b/doc/source/spelling_wordlist.txt @@ -2050,6 +2050,7 @@ nRecurseDepth nRefCount nReqOrder nSampleStep +nScopeFlags nSecond nSize nSrcBufferAllocSize diff --git a/doc/source/user/multithreading.rst b/doc/source/user/multithreading.rst index a4263362a679..c4318bada342 100644 --- a/doc/source/user/multithreading.rst +++ b/doc/source/user/multithreading.rst @@ -4,8 +4,8 @@ Multi-threading =============== -GDAL API: re-entrant, but not thread-safe ------------------------------------------ +GDAL API: re-entrant, but (generally) not thread-safe +----------------------------------------------------- The exact meaning of the terms ``thread-safe`` or ``re-entrant`` is not fully standardized. We will use here the `QT definitions `__. @@ -43,6 +43,26 @@ that are not thread-safe. Those restrictions apply to the C and C++ ABI, and all languages bindings (unless they would take special precautions to serialize calls) +Thread-safe GDAL dataset instances for raster read-only use cases +----------------------------------------------------------------- + +.. versionadded:: 3.10 + +RFC 101 adds a new capability to open, or obtain, a thread-safe dataset from +any dataset, but only for raster read-only use cases. + +At open time, this can be done by passing ``GDAL_OF_RASTER | GDAL_OF_THREAD_SAFE`` +to :cpp:func:`GDALOpenEx` / :cpp:func:`GDALDataset::Open`. + +Given an existing GDALDataset* instance, :cpp:func:`GDALDataset::IsThreadSafe` +can be used to determine if it is thread-safe or not. If not, +:cpp:func:`GDALDataset::GetThreadSafeDataset` can be used. + +Note that the generic implementation of this capability involves opening one +dataset the first time a thread-safe dataset/raster band is accessed by a thread. +While this is an implementation detail that can be ignored to develop code, it is +important to note regarding potential performance impacts + GDAL block cache and multi-threading ------------------------------------ diff --git a/frmts/mem/memdataset.cpp b/frmts/mem/memdataset.cpp index 2d624dafeab4..bf1502a42ea5 100644 --- a/frmts/mem/memdataset.cpp +++ b/frmts/mem/memdataset.cpp @@ -89,10 +89,10 @@ GDALRasterBandH MEMCreateRasterBandEx(GDALDataset *poDS, int nBand, /************************************************************************/ MEMRasterBand::MEMRasterBand(GByte *pabyDataIn, GDALDataType eTypeIn, - int nXSizeIn, int nYSizeIn) + int nXSizeIn, int nYSizeIn, bool bOwnDataIn) : GDALPamRasterBand(FALSE), pabyData(pabyDataIn), nPixelOffset(GDALGetDataTypeSizeBytes(eTypeIn)), nLineOffset(0), - bOwnData(true) + bOwnData(bOwnDataIn) { eAccess = GA_Update; eDataType = eTypeIn; @@ -463,7 +463,7 @@ int MEMRasterBand::GetOverviewCount() MEMDataset *poMemDS = dynamic_cast(poDS); if (poMemDS == nullptr) return 0; - return poMemDS->m_nOverviewDSCount; + return static_cast(poMemDS->m_apoOverviewDS.size()); } /************************************************************************/ @@ -476,9 +476,9 @@ GDALRasterBand *MEMRasterBand::GetOverview(int i) MEMDataset *poMemDS = dynamic_cast(poDS); if (poMemDS == nullptr) return nullptr; - if (i < 0 || i >= poMemDS->m_nOverviewDSCount) + if (i < 0 || i >= static_cast(poMemDS->m_apoOverviewDS.size())) return nullptr; - return poMemDS->m_papoOverviewDS[i]->GetRasterBand(nBand); + return poMemDS->m_apoOverviewDS[i]->GetRasterBand(nBand); } /************************************************************************/ @@ -504,8 +504,8 @@ CPLErr MEMRasterBand::CreateMaskBand(int nFlagsIn) return CE_Failure; nMaskFlags = nFlagsIn; - auto poMemMaskBand = - new MEMRasterBand(pabyMaskData, GDT_Byte, nRasterXSize, nRasterYSize); + auto poMemMaskBand = new MEMRasterBand(pabyMaskData, GDT_Byte, nRasterXSize, + nRasterYSize, /* bOwnData= */ true); poMemMaskBand->m_bIsMask = true; poMask.reset(poMemMaskBand, true); if ((nFlagsIn & GMF_PER_DATASET) != 0 && nBand == 1 && poMemDS != nullptr) @@ -542,8 +542,7 @@ bool MEMRasterBand::IsMaskBand() const /************************************************************************/ MEMDataset::MEMDataset() - : GDALDataset(FALSE), bGeoTransformSet(FALSE), m_nOverviewDSCount(0), - m_papoOverviewDS(nullptr), m_poPrivate(new Private()) + : GDALDataset(FALSE), bGeoTransformSet(FALSE), m_poPrivate(new Private()) { adfGeoTransform[0] = 0.0; adfGeoTransform[1] = 1.0; @@ -565,10 +564,6 @@ MEMDataset::~MEMDataset() bSuppressOnClose = true; FlushCache(true); bSuppressOnClose = bSuppressOnCloseBackup; - - for (int i = 0; i < m_nOverviewDSCount; ++i) - delete m_papoOverviewDS[i]; - CPLFree(m_papoOverviewDS); } #if 0 @@ -824,11 +819,7 @@ CPLErr MEMDataset::IBuildOverviews(const char *pszResampling, int nOverviews, if (nOverviews == 0) { // Cleanup existing overviews - for (int i = 0; i < m_nOverviewDSCount; ++i) - delete m_papoOverviewDS[i]; - CPLFree(m_papoOverviewDS); - m_nOverviewDSCount = 0; - m_papoOverviewDS = nullptr; + m_apoOverviewDS.clear(); return CE_None; } @@ -901,7 +892,7 @@ CPLErr MEMDataset::IBuildOverviews(const char *pszResampling, int nOverviews, // Create new overview dataset if needed. if (!bExisting) { - MEMDataset *poOvrDS = new MEMDataset(); + auto poOvrDS = std::make_unique(); poOvrDS->eAccess = GA_Update; poOvrDS->nRasterXSize = (nRasterXSize + panOverviewList[i] - 1) / panOverviewList[i]; @@ -913,14 +904,10 @@ CPLErr MEMDataset::IBuildOverviews(const char *pszResampling, int nOverviews, GetRasterBand(iBand + 1)->GetRasterDataType(); if (poOvrDS->AddBand(eDT, nullptr) != CE_None) { - delete poOvrDS; return CE_Failure; } } - m_nOverviewDSCount++; - m_papoOverviewDS = (GDALDataset **)CPLRealloc( - m_papoOverviewDS, sizeof(GDALDataset *) * m_nOverviewDSCount); - m_papoOverviewDS[m_nOverviewDSCount - 1] = poOvrDS; + m_apoOverviewDS.emplace_back(std::move(poOvrDS)); } } @@ -1053,6 +1040,106 @@ CPLErr MEMDataset::CreateMaskBand(int nFlagsIn) return poFirstBand->CreateMaskBand(nFlagsIn | GMF_PER_DATASET); } +/************************************************************************/ +/* CanBeCloned() */ +/************************************************************************/ + +/** Implements GDALDataset::CanBeCloned() + * + * This method is called by GDALThreadSafeDataset::Create() to determine if + * it is possible to create a thread-safe wrapper for a dataset, which involves + * the ability to Clone() it. + * + * The implementation of this method must be thread-safe. + */ +bool MEMDataset::CanBeCloned(int nScopeFlags, bool bCanShareState) const +{ + return nScopeFlags == GDAL_OF_RASTER && bCanShareState && + typeid(this) == typeid(const MEMDataset *); +} + +/************************************************************************/ +/* Clone() */ +/************************************************************************/ + +/** Implements GDALDataset::Clone() + * + * This method returns a new instance, identical to "this", but which shares the + * same memory buffer as "this". + * + * The implementation of this method must be thread-safe. + */ +std::unique_ptr MEMDataset::Clone(int nScopeFlags, + bool bCanShareState) const +{ + if (MEMDataset::CanBeCloned(nScopeFlags, bCanShareState)) + { + auto poNewDS = std::make_unique(); + poNewDS->poDriver = poDriver; + poNewDS->nRasterXSize = nRasterXSize; + poNewDS->nRasterYSize = nRasterYSize; + poNewDS->bGeoTransformSet = bGeoTransformSet; + memcpy(poNewDS->adfGeoTransform, adfGeoTransform, + sizeof(adfGeoTransform)); + poNewDS->m_oSRS = m_oSRS; + poNewDS->m_aoGCPs = m_aoGCPs; + poNewDS->m_oGCPSRS = m_oGCPSRS; + for (const auto &poOvrDS : m_apoOverviewDS) + { + poNewDS->m_apoOverviewDS.emplace_back( + poOvrDS->Clone(nScopeFlags, bCanShareState)); + } + + poNewDS->SetDescription(GetDescription()); + poNewDS->oMDMD = oMDMD; + + // Clone bands + for (int i = 1; i <= nBands; ++i) + { + auto poSrcMEMBand = + dynamic_cast(papoBands[i - 1]); + CPLAssert(poSrcMEMBand); + auto poNewBand = std::make_unique( + poNewDS.get(), i, poSrcMEMBand->pabyData, + poSrcMEMBand->GetRasterDataType(), poSrcMEMBand->nPixelOffset, + poSrcMEMBand->nLineOffset, + /* bAssumeOwnership = */ false); + + poNewBand->SetDescription(poSrcMEMBand->GetDescription()); + poNewBand->oMDMD = poSrcMEMBand->oMDMD; + + if (poSrcMEMBand->psPam) + { + poNewBand->PamInitialize(); + CPLAssert(poNewBand->psPam); + poNewBand->psPam->CopyFrom(*(poSrcMEMBand->psPam)); + } + + // Instanciates a mask band when needed. + if ((poSrcMEMBand->nMaskFlags & + (GMF_ALL_VALID | GMF_ALPHA | GMF_NODATA)) == 0) + { + auto poSrcMaskBand = dynamic_cast( + poSrcMEMBand->poMask.get()); + if (poSrcMaskBand) + { + auto poMaskBand = new MEMRasterBand( + poSrcMaskBand->pabyData, GDT_Byte, nRasterXSize, + nRasterYSize, /* bOwnData = */ false); + poMaskBand->m_bIsMask = true; + poNewBand->poMask.reset(poMaskBand, true); + poNewBand->nMaskFlags = poSrcMaskBand->nMaskFlags; + } + } + + poNewDS->SetBand(i, std::move(poNewBand)); + } + + return poNewDS; + } + return GDALDataset::Clone(nScopeFlags, bCanShareState); +} + /************************************************************************/ /* Open() */ /************************************************************************/ diff --git a/frmts/mem/memdataset.h b/frmts/mem/memdataset.h index f2843cbf78dd..8a52c0a69f9d 100644 --- a/frmts/mem/memdataset.h +++ b/frmts/mem/memdataset.h @@ -66,8 +66,7 @@ class CPL_DLL MEMDataset CPL_NON_FINAL : public GDALDataset std::vector m_aoGCPs{}; OGRSpatialReference m_oGCPSRS{}; - int m_nOverviewDSCount; - GDALDataset **m_papoOverviewDS; + std::vector> m_apoOverviewDS{}; struct Private; std::unique_ptr m_poPrivate; @@ -85,6 +84,12 @@ class CPL_DLL MEMDataset CPL_NON_FINAL : public GDALDataset int nYSize, int nBands, GDALDataType eType, char **papszParamList); + protected: + bool CanBeCloned(int nScopeFlags, bool bCanShareState) const override; + + std::unique_ptr Clone(int nScopeFlags, + bool bCanShareState) const override; + public: MEMDataset(); virtual ~MEMDataset(); @@ -141,9 +146,6 @@ class CPL_DLL MEMDataset CPL_NON_FINAL : public GDALDataset class CPL_DLL MEMRasterBand CPL_NON_FINAL : public GDALPamRasterBand { private: - MEMRasterBand(GByte *pabyDataIn, GDALDataType eTypeIn, int nXSizeIn, - int nYSizeIn); - CPL_DISALLOW_COPY_ASSIGN(MEMRasterBand) protected: @@ -156,6 +158,9 @@ class CPL_DLL MEMRasterBand CPL_NON_FINAL : public GDALPamRasterBand bool m_bIsMask = false; + MEMRasterBand(GByte *pabyDataIn, GDALDataType eTypeIn, int nXSizeIn, + int nYSizeIn, bool bOwnDataIn); + public: MEMRasterBand(GDALDataset *poDS, int nBand, GByte *pabyData, GDALDataType eType, GSpacing nPixelOffset, diff --git a/gcore/CMakeLists.txt b/gcore/CMakeLists.txt index 7253fd9cea9e..a629c7f163b2 100644 --- a/gcore/CMakeLists.txt +++ b/gcore/CMakeLists.txt @@ -40,6 +40,7 @@ add_library( gdalrelationship.cpp gdalsubdatasetinfo.cpp gdalorienteddataset.cpp + gdalthreadsafedataset.cpp overview.cpp rasterio.cpp rawdataset.cpp diff --git a/gcore/gdal.h b/gcore/gdal.h index a6ff6d16ca88..33b27eeef877 100644 --- a/gcore/gdal.h +++ b/gcore/gdal.h @@ -1131,6 +1131,14 @@ GDALDatasetH CPL_DLL CPL_STDCALL GDALOpenShared(const char *, GDALAccess) #define GDAL_OF_FROM_GDALOPEN 0x400 #endif +/** Open in thread-safe mode. Not compatible with + * GDAL_OF_VECTOR, GDAL_OF_MULTIDIM_RASTER or GDAL_OF_UPDATE + * + * Used by GDALOpenEx(). + * @since GDAL 3.10 + */ +#define GDAL_OF_THREAD_SAFE 0x800 + GDALDatasetH CPL_DLL CPL_STDCALL GDALOpenEx( const char *pszFilename, unsigned int nOpenFlags, const char *const *papszAllowedDrivers, const char *const *papszOpenOptions, @@ -1241,6 +1249,11 @@ int CPL_DLL CPL_STDCALL GDALGetRasterYSize(GDALDatasetH); int CPL_DLL CPL_STDCALL GDALGetRasterCount(GDALDatasetH); GDALRasterBandH CPL_DLL CPL_STDCALL GDALGetRasterBand(GDALDatasetH, int); +bool CPL_DLL GDALDatasetIsThreadSafe(GDALDatasetH, int nScopeFlags, + CSLConstList papszOptions); +GDALDatasetH CPL_DLL GDALGetThreadSafeDataset(GDALDatasetH, int nScopeFlags, + CSLConstList papszOptions); + CPLErr CPL_DLL CPL_STDCALL GDALAddBand(GDALDatasetH hDS, GDALDataType eType, CSLConstList papszOptions); diff --git a/gcore/gdal_pam.h b/gcore/gdal_pam.h index adb94257550e..e0c0660dd5dc 100644 --- a/gcore/gdal_pam.h +++ b/gcore/gdal_pam.h @@ -269,6 +269,8 @@ struct GDALRasterBandPamInfo bool bOffsetSet = false; bool bScaleSet = false; + + void CopyFrom(const GDALRasterBandPamInfo &sOther); }; //! @endcond diff --git a/gcore/gdal_priv.h b/gcore/gdal_priv.h index 4ee9f5822a6c..9ad11d1bfb89 100644 --- a/gcore/gdal_priv.h +++ b/gcore/gdal_priv.h @@ -141,9 +141,6 @@ class CPL_DLL GDALMultiDomainMetadata { Clear(); } - - private: - CPL_DISALLOW_COPY_ASSIGN(GDALMultiDomainMetadata) }; //! @endcond @@ -620,6 +617,15 @@ class CPL_DLL GDALDataset : public GDALMajorObject void ShareLockWithParentDataset(GDALDataset *poParentDataset); + bool m_bCanBeReopened = false; + + virtual bool CanBeCloned(int nScopeFlags, bool bCanShareState) const; + + friend class GDALThreadSafeDataset; + friend class MEMDataset; + virtual std::unique_ptr Clone(int nScopeFlags, + bool bCanShareState) const; + //! @endcond void CleanupPostFileClosing(); @@ -831,7 +837,7 @@ class CPL_DLL GDALDataset : public GDALMajorObject /** Return MarkSuppressOnClose flag. * @return MarkSuppressOnClose flag. */ - bool IsMarkedSuppressOnClose() + bool IsMarkedSuppressOnClose() const { return bSuppressOnClose; } @@ -844,6 +850,8 @@ class CPL_DLL GDALDataset : public GDALMajorObject return papszOpenOptions; } + bool IsThreadSafe(int nScopeFlags) const; + #ifndef DOXYGEN_SKIP /** Return open options. * @return open options. @@ -4481,6 +4489,12 @@ void CPL_DLL GDALCopyRasterIOExtraArg(GDALRasterIOExtraArg *psDestArg, CPL_C_END +std::unique_ptr CPL_DLL +GDALGetThreadSafeDataset(std::unique_ptr poDS, int nScopeFlags); + +GDALDataset CPL_DLL *GDALGetThreadSafeDataset(GDALDataset *poDS, + int nScopeFlags); + void GDALNullifyOpenDatasetsList(); CPLMutex **GDALGetphDMMutex(); CPLMutex **GDALGetphDLMutex(); diff --git a/gcore/gdaldataset.cpp b/gcore/gdaldataset.cpp index f5a741ee18cd..ee91b2d19302 100644 --- a/gcore/gdaldataset.cpp +++ b/gcore/gdaldataset.cpp @@ -3531,18 +3531,29 @@ static GDALDataset *GetSharedDS(const char *pszFilename, *
  • GDAL_OF_VECTOR for vector drivers,
    • *
  • GDAL_OF_GNM for Geographic Network Model drivers.
    • * - * GDAL_OF_RASTER and GDAL_OF_MULTIDIM_RASTER are generally mutually + * GDAL_OF_RASTER and GDAL_OF_MULTIDIM_RASTER are generally mutually * exclusive. If none of the value is specified, GDAL_OF_RASTER | GDAL_OF_VECTOR - * | GDAL_OF_GNM is implied.
  • Access mode: GDAL_OF_READONLY - * (exclusive)or GDAL_OF_UPDATE.
  • Shared mode: GDAL_OF_SHARED. If set, + * | GDAL_OF_GNM is implied. + *
    • + *
  • Access mode: GDAL_OF_READONLY (exclusive)or GDAL_OF_UPDATE. + *
    • + *
  • Shared mode: GDAL_OF_SHARED. If set, * it allows the sharing of GDALDataset handles for a dataset with other callers * that have set GDAL_OF_SHARED. In particular, GDALOpenEx() will first consult * its list of currently open and shared GDALDataset's, and if the * GetDescription() name for one exactly matches the pszFilename passed to * GDALOpenEx() it will be referenced and returned, if GDALOpenEx() is called - * from the same thread.
  • Verbose error: GDAL_OF_VERBOSE_ERROR. If set, + * from the same thread. + *
    • + *
  • Thread safe mode: GDAL_OF_THREAD_SAFE (added in 3.10). + * This must be use in combination with GDAL_OF_RASTER, and is mutually + * exclusive with GDAL_OF_UPDATE, GDAL_OF_VECTOR, GDAL_OF_MULTIDIM_RASTER or + * GDAL_OF_GNM. + *
    • + *
  • Verbose error: GDAL_OF_VERBOSE_ERROR. If set, * a failed attempt to open the file will lead to an error message to be - * reported.
    • + * reported. + * * * * @param papszAllowedDrivers NULL to consider all candidate drivers, or a NULL @@ -3581,6 +3592,33 @@ GDALDatasetH CPL_STDCALL GDALOpenEx(const char *pszFilename, { VALIDATE_POINTER1(pszFilename, "GDALOpen", nullptr); + // Do some sanity checks on incompatible flags with thread-safe mode. + if ((nOpenFlags & GDAL_OF_THREAD_SAFE) != 0) + { + const struct + { + int nFlag; + const char *pszFlagName; + } asFlags[] = { + {GDAL_OF_UPDATE, "GDAL_OF_UPDATE"}, + {GDAL_OF_VECTOR, "GDAL_OF_VECTOR"}, + {GDAL_OF_MULTIDIM_RASTER, "GDAL_OF_MULTIDIM_RASTER"}, + {GDAL_OF_GNM, "GDAL_OF_GNM"}, + }; + + for (const auto &asFlag : asFlags) + { + if ((nOpenFlags & asFlag.nFlag) != 0) + { + CPLError(CE_Failure, CPLE_IllegalArg, + "GDAL_OF_THREAD_SAFE and %s are mutually " + "exclusive", + asFlag.pszFlagName); + return nullptr; + } + } + } + // If no driver kind is specified, assume all are to be probed. if ((nOpenFlags & GDAL_OF_KIND_MASK) == 0) nOpenFlags |= GDAL_OF_KIND_MASK & ~GDAL_OF_MULTIDIM_RASTER; @@ -3872,7 +3910,12 @@ GDALDatasetH CPL_STDCALL GDALOpenEx(const char *pszFilename, } else { - if (!(nOpenFlags & GDAL_OF_INTERNAL)) + // For thread-safe opening, currently poDS is what will be + // the "master" dataset owned by the thread-safe dataset + // returned to the user, hence we do not register it as a + // visible one in the open dataset list, or mark it as shared. + if (!(nOpenFlags & GDAL_OF_INTERNAL) && + !(nOpenFlags & GDAL_OF_THREAD_SAFE)) { poDS->AddToDatasetOpenList(); } @@ -3881,7 +3924,8 @@ GDALDatasetH CPL_STDCALL GDALOpenEx(const char *pszFilename, CSLDestroy(poDS->papszOpenOptions); poDS->papszOpenOptions = CSLDuplicate(papszOpenOptions); poDS->nOpenFlags = nOpenFlags; - poDS->MarkAsShared(); + if (!(nOpenFlags & GDAL_OF_THREAD_SAFE)) + poDS->MarkAsShared(); } } } @@ -3895,8 +3939,11 @@ GDALDatasetH CPL_STDCALL GDALOpenEx(const char *pszFilename, "and description (%s)", pszFilename, poDS->GetDescription()); } - else + else if (!(nOpenFlags & GDAL_OF_THREAD_SAFE)) { + // For thread-safe opening, currently poDS is what will be + // the "master" dataset owned by the thread-safe dataset + // returned to the user, hence we do not or mark it as shared. poDS->MarkAsShared(); } } @@ -3914,6 +3961,29 @@ GDALDatasetH CPL_STDCALL GDALOpenEx(const char *pszFilename, } #endif + if (poDS) + { + poDS->m_bCanBeReopened = true; + + if ((nOpenFlags & GDAL_OF_THREAD_SAFE) != 0) + { + poDS = + GDALGetThreadSafeDataset( + std::unique_ptr(poDS), GDAL_OF_RASTER) + .release(); + if (poDS) + { + poDS->m_bCanBeReopened = true; + poDS->poDriver = poDriver; + poDS->nOpenFlags = nOpenFlags; + if (!(nOpenFlags & GDAL_OF_INTERNAL)) + poDS->AddToDatasetOpenList(); + if (nOpenFlags & GDAL_OF_SHARED) + poDS->MarkAsShared(); + } + } + } + return poDS; } @@ -8180,7 +8250,8 @@ bool GDALDataset::SetQueryLoggerFunc(CPL_UNUSED GDALQueryLoggerFunc callback, int GDALDataset::EnterReadWrite(GDALRWFlag eRWFlag) { - if (m_poPrivate == nullptr) + if (m_poPrivate == nullptr || + IsThreadSafe(GDAL_OF_RASTER | (nOpenFlags & GDAL_OF_UPDATE))) return FALSE; if (m_poPrivate->poParentDataset) @@ -10136,3 +10207,75 @@ CPLErr GDALDatasetReadCompressedData(GDALDatasetH hDS, const char *pszFormat, pszFormat, nXOff, nYOff, nXSize, nYSize, nBandCount, panBandList, ppBuffer, pnBufferSize, ppszDetailedFormat); } + +/************************************************************************/ +/* CanBeCloned() */ +/************************************************************************/ + +//! @cond Doxygen_Suppress + +/** This method is called by GDALThreadSafeDataset::Create() to determine if + * it is possible to create a thread-safe wrapper for a dataset, which involves + * the ability to Clone() it. + * + * Implementations of this method must be thread-safe. + * + * @param nScopeFlags Combination of GDAL_OF_RASTER, GDAL_OF_VECTOR, etc. flags, + * expressing the intended use for thread-safety. + * Currently, the only valid scope is in the base + * implementation is GDAL_OF_RASTER. + * @param bCanShareState Determines if cloned datasets are allowed to share + * state with the dataset they have been cloned from. + * If set to true, the dataset from which they have been + * cloned from must remain opened during the lifetime of + * its clones. + * @return true if the Clone() method is expecte to succeed with the same values + * of nScopeFlags and bCanShareState. + */ +bool GDALDataset::CanBeCloned(int nScopeFlags, + [[maybe_unused]] bool bCanShareState) const +{ + return m_bCanBeReopened && nScopeFlags == GDAL_OF_RASTER; +} + +//! @endcond + +/************************************************************************/ +/* Clone() */ +/************************************************************************/ + +//! @cond Doxygen_Suppress + +/** This method "clones" the current dataset, that is it returns a new instance + * that is opened on the same underlying "file". + * + * The base implementation uses GDALDataset::Open() to re-open the dataset. + * The MEM driver has a specialized implementation that returns a new instance, + * but which shares the same memory buffer as this. + * + * Implementations of this method must be thread-safe. + * + * @param nScopeFlags Combination of GDAL_OF_RASTER, GDAL_OF_VECTOR, etc. flags, + * expressing the intended use for thread-safety. + * Currently, the only valid scope is in the base + * implementation is GDAL_OF_RASTER. + * @param bCanShareState Determines if cloned datasets are allowed to share + * state with the dataset they have been cloned from. + * If set to true, the dataset from which they have been + * cloned from must remain opened during the lifetime of + * its clones. + * @return a new instance, or nullptr in case of error. + */ +std::unique_ptr +GDALDataset::Clone(int nScopeFlags, [[maybe_unused]] bool bCanShareState) const +{ + CPLStringList aosAllowedDrivers; + if (poDriver) + aosAllowedDrivers.AddString(poDriver->GetDescription()); + return std::unique_ptr(GDALDataset::Open( + GetDescription(), + nScopeFlags | GDAL_OF_INTERNAL | GDAL_OF_VERBOSE_ERROR, + aosAllowedDrivers.List(), papszOpenOptions)); +} + +//! @endcond diff --git a/gcore/gdaldriver.cpp b/gcore/gdaldriver.cpp index c1de9b891886..f801283bdc4a 100644 --- a/gcore/gdaldriver.cpp +++ b/gcore/gdaldriver.cpp @@ -123,7 +123,13 @@ GDALDataset *GDALDriver::Open(GDALOpenInfo *poOpenInfo, bool bSetOpenOptions) if (poDS) { - poDS->nOpenFlags = poOpenInfo->nOpenFlags & ~GDAL_OF_FROM_GDALOPEN; + // Only set GDAL_OF_THREAD_SAFE if the driver itself has set it in + // poDS->nOpenFlags + int nOpenFlags = poOpenInfo->nOpenFlags & + ~(GDAL_OF_FROM_GDALOPEN | GDAL_OF_THREAD_SAFE); + if (poDS->nOpenFlags & GDAL_OF_THREAD_SAFE) + nOpenFlags |= GDAL_OF_THREAD_SAFE; + poDS->nOpenFlags = nOpenFlags; if (strlen(poDS->GetDescription()) == 0) poDS->SetDescription(poOpenInfo->pszFilename); diff --git a/gcore/gdalpamrasterband.cpp b/gcore/gdalpamrasterband.cpp index 670a4d3b38a4..adf8a0ddc4ec 100644 --- a/gcore/gdalpamrasterband.cpp +++ b/gcore/gdalpamrasterband.cpp @@ -50,6 +50,61 @@ #include "gdal_priv.h" #include "gdal_rat.h" +/************************************************************************/ +/* CopyFrom() */ +/************************************************************************/ + +//! @cond Doxygen_Suppress + +void GDALRasterBandPamInfo::CopyFrom(const GDALRasterBandPamInfo &sOther) +{ + bNoDataValueSet = sOther.bNoDataValueSet; + bNoDataValueSetAsInt64 = sOther.bNoDataValueSetAsInt64; + bNoDataValueSetAsUInt64 = sOther.bNoDataValueSetAsUInt64; + + dfNoDataValue = sOther.dfNoDataValue; + nNoDataValueInt64 = sOther.nNoDataValueInt64; + nNoDataValueUInt64 = sOther.nNoDataValueUInt64; + + delete poColorTable; + poColorTable = sOther.poColorTable + ? new GDALColorTable(*(sOther.poColorTable)) + : nullptr; + + eColorInterp = sOther.eColorInterp; + + CPLFree(pszUnitType); + pszUnitType = sOther.pszUnitType ? CPLStrdup(sOther.pszUnitType) : nullptr; + + CSLDestroy(papszCategoryNames); + papszCategoryNames = CSLDuplicate(sOther.papszCategoryNames); + + dfOffset = sOther.dfOffset; + dfScale = sOther.dfScale; + + bHaveMinMax = sOther.bHaveMinMax; + dfMin = sOther.dfMin; + dfMax = sOther.dfMax; + + bHaveStats = sOther.bHaveStats; + dfMean = sOther.dfMean; + dfStdDev = sOther.dfStdDev; + + if (psSavedHistograms) + CPLDestroyXMLNode(psSavedHistograms); + psSavedHistograms = sOther.psSavedHistograms + ? CPLCloneXMLTree(sOther.psSavedHistograms) + : nullptr; + + delete poDefaultRAT; + poDefaultRAT = sOther.poDefaultRAT ? sOther.poDefaultRAT->Clone() : nullptr; + + bOffsetSet = sOther.bOffsetSet; + bScaleSet = sOther.bScaleSet; +} + +//! @endcond + /************************************************************************/ /* GDALPamRasterBand() */ /************************************************************************/ diff --git a/gcore/gdalthreadsafedataset.cpp b/gcore/gdalthreadsafedataset.cpp new file mode 100644 index 000000000000..3b2c8156f249 --- /dev/null +++ b/gcore/gdalthreadsafedataset.cpp @@ -0,0 +1,1169 @@ +/****************************************************************************** + * + * Project: GDAL Core + * Purpose: Base class for thread safe dataset + * Author: Even Rouault + * + ****************************************************************************** + * Copyright (c) 2024, Even Rouault + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + ****************************************************************************/ + +#ifndef DOXYGEN_SKIP + +#include "cpl_mem_cache.h" +#include "gdal_proxy.h" +#include "gdal_rat.h" +#include "gdal_priv.h" + +#include +#include +#include +#include +#include + +/** Design notes of this file. + * + * This file is at the core of the "RFC 101 - Raster dataset read-only thread-safety". + * Please consult it for high level understanding. + * + * 3 classes are involved: + * - GDALThreadSafeDataset whose instances are returned to the user, and can + * use them in a thread-safe way. + * - GDALThreadSafeRasterBand whose instances are created (and owned) by a + * GDALThreadSafeDataset instance, and returned to the user, which can use + * them in a thread-safe way. + * - GDALThreadLocalDatasetCache which is an internal class, which holds the + * thread-local datasets. + */ + +/************************************************************************/ +/* GDALThreadLocalDatasetCache */ +/************************************************************************/ + +class GDALThreadSafeDataset; + +/** This class is instantiated once per thread that uses a + * GDALThreadSafeDataset instance. It holds mostly a cache that maps a + * GDALThreadSafeDataset* pointer to the corresponding per-thread dataset. + */ +class GDALThreadLocalDatasetCache +{ + private: + /** Least-recently-used based cache that maps a GDALThreadSafeDataset* + * instance to the corresponding per-thread dataset. + * It should be noted as this a LRU cache, entries might get evicted when + * its capacity is reached (64 datasets), which might be undesirable. + * Hence it is doubled with m_oMapReferencedDS for datasets that are in + * active used by a thread. + * + * This cache is created as a unique_ptr, and not a standard object, for + * delicate reasons related to application termination, where we might + * want to leak the memory associated to it, to avoid the dataset it + * references from being closed, after GDAL has been "closed" (typically + * GDALDestroyDriverManager() has been called), which would otherwise lead + * to crashes. + */ + std::unique_ptr>> + m_poCache{}; + + GDALThreadLocalDatasetCache(const GDALThreadLocalDatasetCache &) = delete; + GDALThreadLocalDatasetCache & + operator=(const GDALThreadLocalDatasetCache &) = delete; + + public: + GDALThreadLocalDatasetCache(); + ~GDALThreadLocalDatasetCache(); + + /** Thread-id of the thread that instantiated this object. Used only for + * CPLDebug() purposes + */ + GIntBig m_nThreadID = 0; + + /** Mutex that protects access to m_oCache. There is "competition" around + * access to m_oCache since the destructor of a GDALThreadSafeDataset + * instance needs to evict entries corresponding to itself from all + * GDALThreadLocalDatasetCache instances. + */ + std::mutex m_oMutex{}; + + /** This is a reference to *(m_poCache.get()). + */ + lru11::Cache> + &m_oCache; + + /** Pair of shared_ptr with associated thread-local config + * options that were valid in the calling thread at the time + * GDALThreadLocalDatasetCache::RefUnderlyingDataset() was called, so they + * can be restored at UnrefUnderlyingDataset() time. + */ + struct SharedPtrDatasetThreadLocalConfigOptionsPair + { + std::shared_ptr poDS; + CPLStringList aosTLConfigOptions; + + SharedPtrDatasetThreadLocalConfigOptionsPair( + const std::shared_ptr &poDSIn, + CPLStringList &&aosTLConfigOptionsIn) + : poDS(poDSIn), aosTLConfigOptions(std::move(aosTLConfigOptionsIn)) + { + } + }; + + /** Maps a GDALThreadSafeDataset* + * instance to the corresponding per-thread dataset. Insertion into this + * map is done by GDALThreadLocalDatasetCache::RefUnderlyingDataset() and + * removal by UnrefUnderlyingDataset(). In most all use cases, the size of + * this map should be 0 or 1 (not clear if it could be more, that would + * involve RefUnderlyingDataset() being called in nested ways by the same + * thread, but it doesn't hurt from being robust to that potential situation) + */ + std::map + m_oMapReferencedDS{}; + + /** Maps a GDALRasterBand* returned by GDALThreadSafeRasterBand::RefUnderlyingDataset() + * to the (thread-local) dataset that owns it (that is a dataset returned + * by RefUnderlyingDataset(). The size of his map should be 0 or 1 in + * most cases. + */ + std::map m_oMapReferencedDSFromBand{}; +}; + +/************************************************************************/ +/* GDALThreadSafeDataset */ +/************************************************************************/ + +/** Global variable used to determine if the singleton of GlobalCache + * owned by GDALThreadSafeDataset is valid. + * This is needed to avoid issues at process termination where the order + * of destruction between static global instances and TLS instances can be + * tricky. + */ +static bool bGlobalCacheValid = false; + +/** Thread-safe GDALDataset class. + * + * That class delegates all calls to its members to per-thread GDALDataset + * instances. + */ +class GDALThreadSafeDataset final : public GDALProxyDataset +{ + public: + GDALThreadSafeDataset(std::unique_ptr poPrototypeDSUniquePtr, + GDALDataset *poPrototypeDS); + ~GDALThreadSafeDataset() override; + + static std::unique_ptr + Create(std::unique_ptr poPrototypeDS, int nScopeFlags); + + static GDALDataset *Create(GDALDataset *poPrototypeDS, int nScopeFlags); + + protected: + GDALDataset *RefUnderlyingDataset() const override; + + void + UnrefUnderlyingDataset(GDALDataset *poUnderlyingDataset) const override; + + int CloseDependentDatasets() override; + + private: + friend class GDALThreadSafeRasterBand; + friend class GDALThreadLocalDatasetCache; + + /** Mutex that protects accesses to m_poPrototypeDS */ + std::mutex m_oPrototypeDSMutex{}; + + /** "Prototype" dataset, that is the dataset that was passed to the + * GDALThreadSafeDataset constructor. All calls on to it should be on + * const methods, and should be protected by m_oPrototypeDSMutex (except + * during GDALThreadSafeDataset instance construction) + */ + const GDALDataset *m_poPrototypeDS = nullptr; + + /** Unique pointer for m_poPrototypeDS in the cases where GDALThreadSafeDataset + * has been passed a unique pointer */ + std::unique_ptr m_poPrototypeDSUniquePtr{}; + + /** Thread-local config options at the time where GDALThreadSafeDataset + * has been constructed. + */ + const CPLStringList m_aosThreadLocalConfigOptions{}; + + /** Structure that references all GDALThreadLocalDatasetCache* instances. + */ + struct GlobalCache + { + /** Mutex that protect access to oSetOfCache */ + std::mutex oMutex{}; + + /** Set of GDALThreadLocalDatasetCache* instances. That is it has + * one entry per thread that has used at least once a + * GDALThreadLocalDatasetCache/GDALThreadSafeRasterBand + */ + std::set oSetOfCache{}; + + GlobalCache() + { + bGlobalCacheValid = true; + } + + ~GlobalCache() + { + bGlobalCacheValid = false; + } + }; + + /** Returns a singleton for a GlobalCache instance that references all + * GDALThreadLocalDatasetCache* instances. + */ + static GlobalCache &GetSetOfCache() + { + static GlobalCache cache; + return cache; + } + + /** Thread-local dataset cache. */ + static thread_local std::unique_ptr tl_poCache; + + void UnrefUnderlyingDataset(GDALDataset *poUnderlyingDataset, + GDALThreadLocalDatasetCache *poCache) const; + + GDALThreadSafeDataset(const GDALThreadSafeDataset &) = delete; + GDALThreadSafeDataset &operator=(const GDALThreadSafeDataset &) = delete; +}; + +/************************************************************************/ +/* GDALThreadSafeRasterBand */ +/************************************************************************/ + +/** Thread-safe GDALRasterBand class. + * + * That class delegates all calls to its members to per-thread GDALDataset + * instances. + */ +class GDALThreadSafeRasterBand final : public GDALProxyRasterBand +{ + public: + GDALThreadSafeRasterBand(GDALThreadSafeDataset *poTSDS, + GDALDataset *poParentDS, int nBandIn, + GDALRasterBand *poPrototypeBand, + int nBaseBandOfMaskBand, int nOvrIdx); + + GDALRasterBand *GetMaskBand() override; + int GetOverviewCount() override; + GDALRasterBand *GetOverview(int idx) override; + GDALRasterBand *GetRasterSampleOverview(GUIntBig nDesiredSamples) override; + + GDALRasterAttributeTable *GetDefaultRAT() override; + + protected: + GDALRasterBand *RefUnderlyingRasterBand(bool bForceOpen) const override; + void UnrefUnderlyingRasterBand( + GDALRasterBand *poUnderlyingRasterBand) const override; + + private: + /** Pointer to the thread-safe dataset from which this band has been + *created */ + GDALThreadSafeDataset *m_poTSDS = nullptr; + + /** Pointer to the "prototype" raster band that corresponds to us. + * All calls to m_poPrototypeBand should be protected by + * GDALThreadSafeDataset:m_oPrototypeDSMutex. + */ + const GDALRasterBand *m_poPrototypeBand = nullptr; + + /** 0 for standard bands, otherwise > 0 value that indicates that this + * band is a mask band and m_nBaseBandOfMaskBand is then the number + * of the band that is the parent of the mask band. + */ + const int m_nBaseBandOfMaskBand; + + /** 0 for standard bands, otherwise >= 0 value that indicates that this + * band is an overview band and m_nOvrIdx is then the index of the overview. + */ + const int m_nOvrIdx; + + /** Mask band associated with this band. */ + std::unique_ptr m_poMaskBand{}; + + /** List of overviews associated with this band. */ + std::vector> m_apoOverviews{}; + + GDALThreadSafeRasterBand(const GDALThreadSafeRasterBand &) = delete; + GDALThreadSafeRasterBand & + operator=(const GDALThreadSafeRasterBand &) = delete; +}; + +/************************************************************************/ +/* Global variables initialization. */ +/************************************************************************/ + +/** Instantiation of the TLS cache of datasets */ +thread_local std::unique_ptr + GDALThreadSafeDataset::tl_poCache; + +/************************************************************************/ +/* GDALThreadLocalDatasetCache() */ +/************************************************************************/ + +/** Constructor of GDALThreadLocalDatasetCache. This is called implicitly + * when GDALThreadSafeDataset::tl_poCache is called the first time by a + * thread. + */ +GDALThreadLocalDatasetCache::GDALThreadLocalDatasetCache() + : m_poCache(std::make_unique>>()), + m_nThreadID(CPLGetPID()), m_oCache(*m_poCache.get()) +{ + CPLDebug("GDAL", + "Registering thread-safe dataset cache for thread " CPL_FRMT_GIB, + m_nThreadID); + + // We reference ourselves to the GDALThreadSafeDataset set-of-cache singleton + auto &oSetOfCache = GDALThreadSafeDataset::GetSetOfCache(); + std::lock_guard oLock(oSetOfCache.oMutex); + oSetOfCache.oSetOfCache.insert(this); +} + +/************************************************************************/ +/* ~GDALThreadLocalDatasetCache() */ +/************************************************************************/ + +/** Destructor of GDALThreadLocalDatasetCache. This is called implicitly when a + * thread is terminated. + */ +GDALThreadLocalDatasetCache::~GDALThreadLocalDatasetCache() +{ + // If GDAL has been de-initialized explicitly (ie GDALDestroyDriverManager() + // has been called), or we are during process termination, do not try to + // free m_poCache at all, which would cause the datasets its owned to be + // destroyed, which will generally lead to crashes in those situations where + // GDAL has been de-initialized. + const bool bDriverManagerDestroyed = *GDALGetphDMMutex() == nullptr; + if (bDriverManagerDestroyed || !bGlobalCacheValid) + { + // Leak datasets when GDAL has been de-initialized + if (!m_poCache->empty()) + CPL_IGNORE_RET_VAL(m_poCache.release()); + return; + } + + // Unreference ourselves from the GDALThreadSafeDataset set-of-cache singleton + CPLDebug("GDAL", + "Unregistering thread-safe dataset cache for thread " CPL_FRMT_GIB, + m_nThreadID); + { + auto &oSetOfCache = GDALThreadSafeDataset::GetSetOfCache(); + std::lock_guard oLock(oSetOfCache.oMutex); + oSetOfCache.oSetOfCache.erase(this); + } + + // Below code is just for debugging purposes and show which internal + // thread-local datasets are released at thread termination. + const auto lambda = + [this](const lru11::KeyValuePair> &kv) + { + CPLDebug("GDAL", + "~GDALThreadLocalDatasetCache(): GDALClose(%s, this=%p) " + "for thread " CPL_FRMT_GIB, + kv.value->GetDescription(), kv.value.get(), m_nThreadID); + }; + m_oCache.cwalk(lambda); +} + +/************************************************************************/ +/* GDALThreadSafeDataset() */ +/************************************************************************/ + +/** Constructor of GDALThreadSafeDataset. + * It may be called with poPrototypeDSUniquePtr set to null, in the situations + * where GDALThreadSafeDataset doesn't own the prototype dataset. + * poPrototypeDS should always be not-null, and if poPrototypeDSUniquePtr is + * not null, then poPrototypeDS should be equal to poPrototypeDSUniquePtr.get() + */ +GDALThreadSafeDataset::GDALThreadSafeDataset( + std::unique_ptr poPrototypeDSUniquePtr, + GDALDataset *poPrototypeDS) + : m_poPrototypeDS(poPrototypeDS), + m_aosThreadLocalConfigOptions(CPLGetThreadLocalConfigOptions()) +{ + CPLAssert(poPrototypeDS != nullptr); + if (poPrototypeDSUniquePtr) + { + CPLAssert(poPrototypeDS == poPrototypeDSUniquePtr.get()); + } + + // Replicate the characteristics of the prototype dataset onto ourselves + nRasterXSize = poPrototypeDS->GetRasterXSize(); + nRasterYSize = poPrototypeDS->GetRasterYSize(); + for (int i = 1; i <= poPrototypeDS->GetRasterCount(); ++i) + { + SetBand(i, std::make_unique( + this, this, i, poPrototypeDS->GetRasterBand(i), 0, -1)); + } + nOpenFlags = GDAL_OF_RASTER | GDAL_OF_THREAD_SAFE; + SetDescription(poPrototypeDS->GetDescription()); + papszOpenOptions = CSLDuplicate(poPrototypeDS->GetOpenOptions()); + + m_poPrototypeDSUniquePtr = std::move(poPrototypeDSUniquePtr); + + // In the case where we are constructed without owning the prototype + // dataset, let's increase its reference counter though. + if (!m_poPrototypeDSUniquePtr) + const_cast(m_poPrototypeDS)->Reference(); +} + +/************************************************************************/ +/* Create() */ +/************************************************************************/ + +/** Utility method used by GDALGetThreadSafeDataset() to construct a + * GDALThreadSafeDataset instance in the case where the GDALThreadSafeDataset + * instance owns the prototype dataset. + */ + +/* static */ std::unique_ptr +GDALThreadSafeDataset::Create(std::unique_ptr poPrototypeDS, + int nScopeFlags) +{ + if (nScopeFlags != GDAL_OF_RASTER) + { + CPLError(CE_Failure, CPLE_NotSupported, + "GDALGetThreadSafeDataset(): Only nScopeFlags == " + "GDAL_OF_RASTER is supported"); + return nullptr; + } + if (poPrototypeDS->IsThreadSafe(nScopeFlags)) + { + return poPrototypeDS; + } + if (!poPrototypeDS->CanBeCloned(nScopeFlags, /* bCanShareState = */ true)) + { + CPLError(CE_Failure, CPLE_NotSupported, + "GDALGetThreadSafeDataset(): Source dataset cannot be " + "cloned"); + return nullptr; + } + auto poPrototypeDSRaw = poPrototypeDS.get(); + return std::make_unique(std::move(poPrototypeDS), + poPrototypeDSRaw); +} + +/************************************************************************/ +/* Create() */ +/************************************************************************/ + +/** Utility method used by GDALGetThreadSafeDataset() to construct a + * GDALThreadSafeDataset instance in the case where the GDALThreadSafeDataset + * instance does not own the prototype dataset. + */ + +/* static */ GDALDataset * +GDALThreadSafeDataset::Create(GDALDataset *poPrototypeDS, int nScopeFlags) +{ + if (nScopeFlags != GDAL_OF_RASTER) + { + CPLError(CE_Failure, CPLE_NotSupported, + "GDALGetThreadSafeDataset(): Only nScopeFlags == " + "GDAL_OF_RASTER is supported"); + return nullptr; + } + if (poPrototypeDS->IsThreadSafe(nScopeFlags)) + { + poPrototypeDS->Reference(); + return poPrototypeDS; + } + if (!poPrototypeDS->CanBeCloned(nScopeFlags, /* bCanShareState = */ true)) + { + CPLError(CE_Failure, CPLE_NotSupported, + "GDALGetThreadSafeDataset(): Source dataset cannot be " + "cloned"); + return nullptr; + } + return std::make_unique(nullptr, poPrototypeDS) + .release(); +} + +/************************************************************************/ +/* ~GDALThreadSafeDataset() */ +/************************************************************************/ + +GDALThreadSafeDataset::~GDALThreadSafeDataset() +{ + // Collect TLS datasets in a vector, and free them after releasing + // g_nInDestructorCounter to limit contention + std::vector, GIntBig>> aoDSToFree; + { + auto &oSetOfCache = GetSetOfCache(); + std::lock_guard oLock(oSetOfCache.oMutex); + for (auto *poCache : oSetOfCache.oSetOfCache) + { + std::unique_lock oLockCache(poCache->m_oMutex); + std::shared_ptr poDS; + if (poCache->m_oCache.tryGet(this, poDS)) + { + aoDSToFree.emplace_back(std::move(poDS), poCache->m_nThreadID); + poCache->m_oCache.remove(this); + } + } + } + + for (const auto &oEntry : aoDSToFree) + { + CPLDebug("GDAL", + "~GDALThreadSafeDataset(): GDALClose(%s, this=%p) for " + "thread " CPL_FRMT_GIB, + GetDescription(), oEntry.first.get(), oEntry.second); + } + // Actually release TLS datasets + aoDSToFree.clear(); + + GDALThreadSafeDataset::CloseDependentDatasets(); +} + +/************************************************************************/ +/* CloseDependentDatasets() */ +/************************************************************************/ + +/** Implements GDALDataset::CloseDependentDatasets() + * + * Takes care of releasing the prototype dataset. + * + * As implied by the contract of CloseDependentDatasets(), returns true if + * the prototype dataset has actually been released (or false if + * CloseDependentDatasets() has already been closed) + */ +int GDALThreadSafeDataset::CloseDependentDatasets() +{ + int bRet = false; + if (m_poPrototypeDSUniquePtr) + { + bRet = true; + } + else if (m_poPrototypeDS) + { + if (const_cast(m_poPrototypeDS)->ReleaseRef()) + { + bRet = true; + } + } + + m_poPrototypeDSUniquePtr.reset(); + m_poPrototypeDS = nullptr; + + return bRet; +} + +/************************************************************************/ +/* RefUnderlyingDataset() */ +/************************************************************************/ + +/** Implements GDALProxyDataset::RefUnderlyingDataset. + * + * This method is called by all virtual methods of GDALDataset overridden by + * RefUnderlyingDataset() when it delegates the calls to the underlying + * dataset. + * + * Our implementation takes care of opening a thread-local dataset, on the + * same underlying dataset of m_poPrototypeDS, if needed, and to insert it + * into a cache for fast later uses by the same thread. + */ +GDALDataset *GDALThreadSafeDataset::RefUnderlyingDataset() const +{ + // Back-up thread-local config options at the time we are called + CPLStringList aosTLConfigOptionsBackup(CPLGetThreadLocalConfigOptions()); + + // Now merge the thread-local config options at the time where this + // instance has been created with the current ones. + const CPLStringList aosMerged( + CSLMerge(CSLDuplicate(m_aosThreadLocalConfigOptions.List()), + aosTLConfigOptionsBackup.List())); + + // And make that merged list active + CPLSetThreadLocalConfigOptions(aosMerged.List()); + + std::shared_ptr poTLSDS; + + // Get the thread-local dataset cache for this thread. + GDALThreadLocalDatasetCache *poCache = tl_poCache.get(); + if (!poCache) + { + auto poCacheUniquePtr = std::make_unique(); + poCache = poCacheUniquePtr.get(); + tl_poCache = std::move(poCacheUniquePtr); + } + + // Check if there's an entry in this cache for our current GDALThreadSafeDataset + // instance. + std::unique_lock oLock(poCache->m_oMutex); + if (poCache->m_oCache.tryGet(this, poTLSDS)) + { + // If so, return it, but before returning, make sure to creates a + // "hard" reference to the thread-local dataset, in case it would + // get evicted from poCache->m_oCache (by other threads that would + // access lots of datasets in between) + CPLAssert(!cpl::contains(poCache->m_oMapReferencedDS, this)); + auto poDSRet = poTLSDS.get(); + poCache->m_oMapReferencedDS.insert( + {this, GDALThreadLocalDatasetCache:: + SharedPtrDatasetThreadLocalConfigOptionsPair( + poTLSDS, std::move(aosTLConfigOptionsBackup))}); + return poDSRet; + } + + // "Clone" the prototype dataset, which in 99% of the cases, involves + // doing a GDALDataset::Open() call to re-open it. Do that by temporarily + // dropping the lock that protects poCache->m_oCache. + oLock.unlock(); + poTLSDS = m_poPrototypeDS->Clone(GDAL_OF_RASTER, /* bCanShareState=*/true); + if (poTLSDS) + { + CPLDebug("GDAL", "GDALOpen(%s, this=%p) for thread " CPL_FRMT_GIB, + GetDescription(), poTLSDS.get(), CPLGetPID()); + + // Check that the re-openeded dataset has the same characteristics + // as "this" / m_poPrototypeDS + if (poTLSDS->GetRasterXSize() != nRasterXSize || + poTLSDS->GetRasterYSize() != nRasterYSize || + poTLSDS->GetRasterCount() != nBands) + { + poTLSDS.reset(); + CPLError(CE_Failure, CPLE_AppDefined, + "Re-opened dataset for %s does not share the same " + "characteristics has the master dataset", + GetDescription()); + } + } + + // Re-acquire the lok + oLock.lock(); + + // In case of failed closing, restore the thread-local config options that + // were valid at the beginning of this method, and return in error. + if (!poTLSDS) + { + CPLSetThreadLocalConfigOptions(aosTLConfigOptionsBackup.List()); + return nullptr; + } + + // We have managed to get a thread-local dataset. Insert it into the + // LRU cache and the m_oMapReferencedDS map that holds strong references. + auto poDSRet = poTLSDS.get(); + { + poCache->m_oCache.insert(this, poTLSDS); + CPLAssert(!cpl::contains(poCache->m_oMapReferencedDS, this)); + poCache->m_oMapReferencedDS.insert( + {this, GDALThreadLocalDatasetCache:: + SharedPtrDatasetThreadLocalConfigOptionsPair( + poTLSDS, std::move(aosTLConfigOptionsBackup))}); + } + return poDSRet; +} + +/************************************************************************/ +/* UnrefUnderlyingDataset() */ +/************************************************************************/ + +/** Implements GDALProxyDataset::UnrefUnderlyingDataset. + * + * This is called by GDALProxyDataset overridden methods of GDALDataset, when + * they no longer need to access the underlying dataset. + * + * This method actually delegates most of the work to the other + * UnrefUnderlyingDataset() method that takes an explicit GDALThreadLocalDatasetCache* + * instance. + */ +void GDALThreadSafeDataset::UnrefUnderlyingDataset( + GDALDataset *poUnderlyingDataset) const +{ + GDALThreadLocalDatasetCache *poCache = tl_poCache.get(); + CPLAssert(poCache); + std::unique_lock oLock(poCache->m_oMutex); + UnrefUnderlyingDataset(poUnderlyingDataset, poCache); +} + +/************************************************************************/ +/* UnrefUnderlyingDataset() */ +/************************************************************************/ + +/** Takes care of removing the strong reference to a thread-local dataset + * from the TLS cache of datasets. + */ +void GDALThreadSafeDataset::UnrefUnderlyingDataset( + [[maybe_unused]] GDALDataset *poUnderlyingDataset, + GDALThreadLocalDatasetCache *poCache) const +{ + auto oIter = poCache->m_oMapReferencedDS.find(this); + CPLAssert(oIter != poCache->m_oMapReferencedDS.end()); + CPLAssert(oIter->second.poDS.get() == poUnderlyingDataset); + CPLSetThreadLocalConfigOptions(oIter->second.aosTLConfigOptions.List()); + poCache->m_oMapReferencedDS.erase(oIter); +} + +/************************************************************************/ +/* GDALThreadSafeRasterBand() */ +/************************************************************************/ + +GDALThreadSafeRasterBand::GDALThreadSafeRasterBand( + GDALThreadSafeDataset *poTSDS, GDALDataset *poParentDS, int nBandIn, + GDALRasterBand *poPrototypeBand, int nBaseBandOfMaskBand, int nOvrIdx) + : m_poTSDS(poTSDS), m_poPrototypeBand(poPrototypeBand), + m_nBaseBandOfMaskBand(nBaseBandOfMaskBand), m_nOvrIdx(nOvrIdx) +{ + // Replicates characteristics of the prototype band. + poDS = poParentDS; + nBand = nBandIn; + eDataType = poPrototypeBand->GetRasterDataType(); + nRasterXSize = poPrototypeBand->GetXSize(); + nRasterYSize = poPrototypeBand->GetYSize(); + poPrototypeBand->GetBlockSize(&nBlockXSize, &nBlockYSize); + + if (nBandIn > 0) + { + // For regular bands instantiates a (thread-safe) mask band and + // as many overviews as needed. + + m_poMaskBand = std::make_unique( + poTSDS, nullptr, 0, poPrototypeBand->GetMaskBand(), nBandIn, + nOvrIdx); + if (nOvrIdx < 0) + { + const int nOvrCount = poPrototypeBand->GetOverviewCount(); + for (int iOvrIdx = 0; iOvrIdx < nOvrCount; ++iOvrIdx) + { + m_apoOverviews.emplace_back( + std::make_unique( + poTSDS, nullptr, nBandIn, + poPrototypeBand->GetOverview(iOvrIdx), + nBaseBandOfMaskBand, iOvrIdx)); + } + } + } + else if (nBaseBandOfMaskBand > 0) + { + // If we are a mask band, nstanciates a (thread-safe) mask band of + // ourselves, but with the trick of negating nBaseBandOfMaskBand to + // avoid infinite recursion... + m_poMaskBand = std::make_unique( + poTSDS, nullptr, 0, poPrototypeBand->GetMaskBand(), + -nBaseBandOfMaskBand, nOvrIdx); + } +} + +/************************************************************************/ +/* RefUnderlyingRasterBand() */ +/************************************************************************/ + +/** Implements GDALProxyRasterBand::RefUnderlyingDataset. + * + * This method is called by all virtual methods of GDALRasterBand overridden by + * RefUnderlyingRasterBand() when it delegates the calls to the underlying + * band. + */ +GDALRasterBand * +GDALThreadSafeRasterBand::RefUnderlyingRasterBand(bool /*bForceOpen*/) const +{ + // Get a thread-local dataset + auto poTLDS = m_poTSDS->RefUnderlyingDataset(); + if (!poTLDS) + return nullptr; + + // Get corresponding thread-local band. If m_nBaseBandOfMaskBand is not + // zero, then the base band is indicated into it, otherwise use nBand. + const int nTLSBandIdx = + m_nBaseBandOfMaskBand ? std::abs(m_nBaseBandOfMaskBand) : nBand; + auto poTLRasterBand = poTLDS->GetRasterBand(nTLSBandIdx); + if (!poTLRasterBand) + { + CPLError(CE_Failure, CPLE_AppDefined, + "GDALThreadSafeRasterBand::RefUnderlyingRasterBand(): " + "GetRasterBand(%d) failed", + nTLSBandIdx); + m_poTSDS->UnrefUnderlyingDataset(poTLDS); + return nullptr; + } + + // Get the overview level if needed. + if (m_nOvrIdx >= 0) + { + poTLRasterBand = poTLRasterBand->GetOverview(m_nOvrIdx); + if (!poTLRasterBand) + { + CPLError(CE_Failure, CPLE_AppDefined, + "GDALThreadSafeRasterBand::RefUnderlyingRasterBand(): " + "GetOverview(%d) failed", + m_nOvrIdx); + m_poTSDS->UnrefUnderlyingDataset(poTLDS); + return nullptr; + } + } + + // Get the mask band (or the mask band of the mask band) if needed. + if (m_nBaseBandOfMaskBand) + { + poTLRasterBand = poTLRasterBand->GetMaskBand(); + if (m_nBaseBandOfMaskBand < 0) + poTLRasterBand = poTLRasterBand->GetMaskBand(); + } + + // Check that the thread-local band characteristics are identical to the + // ones of the prototype band. + if (m_poPrototypeBand->GetXSize() != poTLRasterBand->GetXSize() || + m_poPrototypeBand->GetYSize() != poTLRasterBand->GetYSize() || + m_poPrototypeBand->GetRasterDataType() != + poTLRasterBand->GetRasterDataType() + // m_poPrototypeBand->GetMaskFlags() is not thread-safe + // || m_poPrototypeBand->GetMaskFlags() != poTLRasterBand->GetMaskFlags() + ) + { + CPLError(CE_Failure, CPLE_AppDefined, + "GDALThreadSafeRasterBand::RefUnderlyingRasterBand(): TLS " + "band has not expected characteristics"); + m_poTSDS->UnrefUnderlyingDataset(poTLDS); + return nullptr; + } + int nThisBlockXSize; + int nThisBlockYSize; + int nTLSBlockXSize; + int nTLSBlockYSize; + m_poPrototypeBand->GetBlockSize(&nThisBlockXSize, &nThisBlockYSize); + poTLRasterBand->GetBlockSize(&nTLSBlockXSize, &nTLSBlockYSize); + if (nThisBlockXSize != nTLSBlockXSize || nThisBlockYSize != nTLSBlockYSize) + { + CPLError(CE_Failure, CPLE_AppDefined, + "GDALThreadSafeRasterBand::RefUnderlyingRasterBand(): TLS " + "band has not expected characteristics"); + m_poTSDS->UnrefUnderlyingDataset(poTLDS); + return nullptr; + } + + // Registers the association between the thread-local band and the + // thread-local dataset + { + GDALThreadLocalDatasetCache *poCache = + GDALThreadSafeDataset::tl_poCache.get(); + CPLAssert(poCache); + std::unique_lock oLock(poCache->m_oMutex); + CPLAssert(!cpl::contains(poCache->m_oMapReferencedDSFromBand, + poTLRasterBand)); + poCache->m_oMapReferencedDSFromBand[poTLRasterBand] = poTLDS; + } + // CPLDebug("GDAL", "%p->RefUnderlyingRasterBand() return %p", this, poTLRasterBand); + return poTLRasterBand; +} + +/************************************************************************/ +/* UnrefUnderlyingRasterBand() */ +/************************************************************************/ + +/** Implements GDALProxyRasterBand::UnrefUnderlyingRasterBand. + * + * This is called by GDALProxyRasterBand overridden methods of GDALRasterBand, + * when they no longer need to access the underlying dataset. + */ +void GDALThreadSafeRasterBand::UnrefUnderlyingRasterBand( + GDALRasterBand *poUnderlyingRasterBand) const +{ + // CPLDebug("GDAL", "%p->UnrefUnderlyingRasterBand(%p)", this, poUnderlyingRasterBand); + + // Unregisters the association between the thread-local band and the + // thread-local dataset + { + GDALThreadLocalDatasetCache *poCache = + GDALThreadSafeDataset::tl_poCache.get(); + CPLAssert(poCache); + std::unique_lock oLock(poCache->m_oMutex); + auto oIter = + poCache->m_oMapReferencedDSFromBand.find(poUnderlyingRasterBand); + CPLAssert(oIter != poCache->m_oMapReferencedDSFromBand.end()); + + m_poTSDS->UnrefUnderlyingDataset(oIter->second, poCache); + poCache->m_oMapReferencedDSFromBand.erase(oIter); + } +} + +/************************************************************************/ +/* GetMaskBand() */ +/************************************************************************/ + +/** Implements GDALRasterBand::GetMaskBand + */ +GDALRasterBand *GDALThreadSafeRasterBand::GetMaskBand() +{ + return m_poMaskBand ? m_poMaskBand.get() : this; +} + +/************************************************************************/ +/* GetOverviewCount() */ +/************************************************************************/ + +/** Implements GDALRasterBand::GetOverviewCount + */ +int GDALThreadSafeRasterBand::GetOverviewCount() +{ + return static_cast(m_apoOverviews.size()); +} + +/************************************************************************/ +/* GetOverview() */ +/************************************************************************/ + +/** Implements GDALRasterBand::GetOverview + */ +GDALRasterBand *GDALThreadSafeRasterBand::GetOverview(int nIdx) +{ + if (nIdx < 0 || nIdx >= static_cast(m_apoOverviews.size())) + return nullptr; + return m_apoOverviews[nIdx].get(); +} + +/************************************************************************/ +/* GetRasterSampleOverview() */ +/************************************************************************/ + +/** Implements GDALRasterBand::GetRasterSampleOverview + */ +GDALRasterBand * +GDALThreadSafeRasterBand::GetRasterSampleOverview(GUIntBig nDesiredSamples) + +{ + // Call the base implementation, and do not forward to proxy + return GDALRasterBand::GetRasterSampleOverview(nDesiredSamples); +} + +/************************************************************************/ +/* GetDefaultRAT() */ +/************************************************************************/ + +/** Implements GDALRasterBand::GetDefaultRAT + * + * This is a bit tricky to do as GDALRasterAttributeTable has virtual methods + * with potential (non thread-safe) side-effects. The clean solution would be + * to implement a GDALThreadSafeRAT wrapper class, but this is a bit too much + * effort. So for now, we check if the RAT returned by the prototype band is + * an instance of GDALDefaultRasterAttributeTable. If it is, given that this + * class has thread-safe getters, we can directly return it. + * Otherwise return in error. + */ +GDALRasterAttributeTable *GDALThreadSafeRasterBand::GetDefaultRAT() +{ + std::lock_guard oGuard(m_poTSDS->m_oPrototypeDSMutex); + const auto poRAT = + const_cast(m_poPrototypeBand)->GetDefaultRAT(); + if (!poRAT) + return nullptr; + + if (dynamic_cast(poRAT)) + return poRAT; + + CPLError(CE_Failure, CPLE_AppDefined, + "GDALThreadSafeRasterBand::GetDefaultRAT() not supporting a " + "non-GDALDefaultRasterAttributeTable implementation"); + return nullptr; +} + +#endif // DOXYGEN_SKIP + +/************************************************************************/ +/* GDALDataset::IsThreadSafe() */ +/************************************************************************/ + +/** Return whether this dataset, and its related objects (typically raster + * bands), can be called for the intended scope. + * + * Note that in the current implementation, nScopeFlags should be set to + * GDAL_OF_RASTER, as thread-safety is limited to read-only operations and + * excludes operations on vector layers (OGRLayer) or multidimensional API + * (GDALGroup, GDALMDArray, etc.) + * + * This is the same as the C function GDALDatasetIsThreadSafe(). + * + * @since 3.10 + */ +bool GDALDataset::IsThreadSafe(int nScopeFlags) const +{ + return (nOpenFlags & GDAL_OF_THREAD_SAFE) != 0 && + nScopeFlags == GDAL_OF_RASTER && (nOpenFlags & GDAL_OF_RASTER) != 0; +} + +/************************************************************************/ +/* GDALDatasetIsThreadSafe() */ +/************************************************************************/ + +/** Return whether this dataset, and its related objects (typically raster + * bands), can be called for the intended scope. + * + * Note that in the current implementation, nScopeFlags should be set to + * GDAL_OF_RASTER, as thread-safety is limited to read-only operations and + * excludes operations on vector layers (OGRLayer) or multidimensional API + * (GDALGroup, GDALMDArray, etc.) + * + * This is the same as the C++ method GDALDataset::IsThreadSafe(). + * + * @param hDS Source dataset + * @param nScopeFlags Intended scope of use. + * Only GDAL_OF_RASTER is supported currently. + * @param papszOptions Options. None currently. + * + * @since 3.10 + */ +bool GDALDatasetIsThreadSafe(GDALDatasetH hDS, int nScopeFlags, + CSLConstList papszOptions) +{ + VALIDATE_POINTER1(hDS, __func__, false); + + CPL_IGNORE_RET_VAL(papszOptions); + + return GDALDataset::FromHandle(hDS)->IsThreadSafe(nScopeFlags); +} + +/************************************************************************/ +/* GDALGetThreadSafeDataset() */ +/************************************************************************/ + +/** Return a thread-safe dataset. + * + * In the general case, this thread-safe dataset will open a + * behind-the-scenes per-thread dataset (re-using the name and open options of poDS), + * the first time a thread calls a method on the thread-safe dataset, and will + * transparently redirect calls from the calling thread to this behind-the-scenes + * per-thread dataset. Hence there is an initial setup cost per thread. + * Datasets of the MEM driver cannot be opened by name, but this function will + * take care of "cloning" them, using the same backing memory, when needed. + * + * Ownership of the passed dataset is transferred to the thread-safe dataset. + * + * The function may also return the passed dataset if it is already thread-safe. + * + * @param poDS Source dataset + * @param nScopeFlags Intended scope of use. + * Only GDAL_OF_RASTER is supported currently. + * + * @return a new thread-safe dataset, or nullptr in case of error. + * + * @since 3.10 + */ +std::unique_ptr +GDALGetThreadSafeDataset(std::unique_ptr poDS, int nScopeFlags) +{ + return GDALThreadSafeDataset::Create(std::move(poDS), nScopeFlags); +} + +/************************************************************************/ +/* GDALGetThreadSafeDataset() */ +/************************************************************************/ + +/** Return a thread-safe dataset. + * + * In the general case, this thread-safe dataset will open a + * behind-the-scenes per-thread dataset (re-using the name and open options of poDS), + * the first time a thread calls a method on the thread-safe dataset, and will + * transparently redirect calls from the calling thread to this behind-the-scenes + * per-thread dataset. Hence there is an initial setup cost per thread. + * Datasets of the MEM driver cannot be opened by name, but this function will + * take care of "cloning" them, using the same backing memory, when needed. + * + * The life-time of the passed dataset must be longer than the one of + * the returned thread-safe dataset. + * + * Note that this function does increase the reference count on poDS while + * it is being used + * + * The function may also return the passed dataset if it is already thread-safe. + * A non-nullptr returned dataset must be released with ReleaseRef(). + * + * Patterns like the following one are valid: + * \code{.cpp} + * auto poDS = GDALDataset::Open(...); + * auto poThreadSafeDS = GDALGetThreadSafeDataset(poDS, GDAL_OF_RASTER | GDAL_OF_THREAD_SAFE); + * poDS->ReleaseRef(); + * if (poThreadSafeDS ) + * { + * // ... do something with poThreadSafeDS ... + * poThreadSafeDS->ReleaseRef(); + * } + * \endcode + * + * @param poDS Source dataset + * @param nScopeFlags Intended scope of use. + * Only GDAL_OF_RASTER is supported currently. + * + * @return a new thread-safe dataset or poDS, or nullptr in case of error. + + * @since 3.10 + */ +GDALDataset *GDALGetThreadSafeDataset(GDALDataset *poDS, int nScopeFlags) +{ + return GDALThreadSafeDataset::Create(poDS, nScopeFlags); +} + +/************************************************************************/ +/* GDALGetThreadSafeDataset() */ +/************************************************************************/ + +/** Return a thread-safe dataset. + * + * In the general case, this thread-safe dataset will open a + * behind-the-scenes per-thread dataset (re-using the name and open options of hDS), + * the first time a thread calls a method on the thread-safe dataset, and will + * transparently redirect calls from the calling thread to this behind-the-scenes + * per-thread dataset. Hence there is an initial setup cost per thread. + * Datasets of the MEM driver cannot be opened by name, but this function will + * take care of "cloning" them, using the same backing memory, when needed. + * + * The life-time of the passed dataset must be longer than the one of + * the returned thread-safe dataset. + * + * Note that this function does increase the reference count on poDS while + * it is being used + * + * The function may also return the passed dataset if it is already thread-safe. + * A non-nullptr returned dataset must be released with GDALReleaseDataset(). + * + * \code{.cpp} + * hDS = GDALOpenEx(...); + * hThreadSafeDS = GDALGetThreadSafeDataset(hDS, GDAL_OF_RASTER | GDAL_OF_THREAD_SAFE, NULL); + * GDALReleaseDataset(hDS); + * if( hThreadSafeDS ) + * { + * // ... do something with hThreadSafeDS ... + * GDALReleaseDataset(hThreadSafeDS); + * } + * \endcode + * + * @param hDS Source dataset + * @param nScopeFlags Intended scope of use. + * Only GDAL_OF_RASTER is supported currently. + * @param papszOptions Options. None currently. + * + * @since 3.10 + */ +GDALDatasetH GDALGetThreadSafeDataset(GDALDatasetH hDS, int nScopeFlags, + CSLConstList papszOptions) +{ + VALIDATE_POINTER1(hDS, __func__, nullptr); + + CPL_IGNORE_RET_VAL(papszOptions); + return GDALDataset::ToHandle( + GDALGetThreadSafeDataset(GDALDataset::FromHandle(hDS), nScopeFlags)); +} diff --git a/ogr/ogrsf_frmts/sqlite/ogrsqlitedatasource.cpp b/ogr/ogrsf_frmts/sqlite/ogrsqlitedatasource.cpp index d6ce19133af9..3e34c28334f7 100644 --- a/ogr/ogrsf_frmts/sqlite/ogrsqlitedatasource.cpp +++ b/ogr/ogrsf_frmts/sqlite/ogrsqlitedatasource.cpp @@ -2165,7 +2165,7 @@ bool OGRSQLiteDataSource::Open(GDALOpenInfo *poOpenInfo) const char *pszNewName = poOpenInfo->pszFilename; CPLAssert(m_apoLayers.empty()); eAccess = poOpenInfo->eAccess; - nOpenFlags = poOpenInfo->nOpenFlags; + nOpenFlags = poOpenInfo->nOpenFlags & ~GDAL_OF_THREAD_SAFE; SetDescription(pszNewName); if (m_pszFilename == nullptr) diff --git a/port/cpl_mem_cache.h b/port/cpl_mem_cache.h index 6fecca4b239f..0e021b762748 100644 --- a/port/cpl_mem_cache.h +++ b/port/cpl_mem_cache.h @@ -86,6 +86,10 @@ template struct KeyValuePair KeyValuePair(const K &k, V &&v) : key(k), value(std::move(v)) { } + + private: + KeyValuePair(const KeyValuePair &) = delete; + KeyValuePair &operator=(const KeyValuePair &) = delete; }; /** diff --git a/swig/include/Band.i b/swig/include/Band.i index 5766f0e0c6cc..000a897f457f 100644 --- a/swig/include/Band.i +++ b/swig/include/Band.i @@ -295,6 +295,10 @@ public: return (GDALRasterBandShadow*) GDALGetOverview( self, i ); } + GDALRasterBandShadow *GetSampleOverview(GUIntBig nDesiredSamples) { + return (GDALRasterBandShadow*) GDALGetRasterSampleOverview( self, nDesiredSamples ); + } + #if defined (SWIGJAVA) int Checksum( int xoff, int yoff, int xsize, int ysize) { return GDALChecksumImage( self, xoff, yoff, xsize, ysize ); diff --git a/swig/include/Dataset.i b/swig/include/Dataset.i index 2f3b2e9b6ab7..a128a66992f9 100644 --- a/swig/include/Dataset.i +++ b/swig/include/Dataset.i @@ -300,6 +300,17 @@ public: return (GDALRasterBandShadow*) GDALGetRasterBand( self, nBand ); } + bool IsThreadSafe(int nScopeFlags) + { + return GDALDatasetIsThreadSafe(self, nScopeFlags, nullptr); + } + +%newobject GetThreadSafeDataset; + GDALDatasetShadow* GetThreadSafeDataset(int nScopeFlags) + { + return GDALGetThreadSafeDataset(self, nScopeFlags, nullptr); + } + %newobject GetRootGroup; GDALGroupHS* GetRootGroup() { return GDALDatasetGetRootGroup(self); diff --git a/swig/include/gdalconst.i b/swig/include/gdalconst.i index 4a664f959e78..c9902d6f1293 100644 --- a/swig/include/gdalconst.i +++ b/swig/include/gdalconst.i @@ -197,6 +197,7 @@ %constant OF_UPDATE = GDAL_OF_UPDATE; %constant OF_SHARED = GDAL_OF_SHARED; %constant OF_VERBOSE_ERROR = GDAL_OF_VERBOSE_ERROR; +%constant OF_THREAD_SAFE = GDAL_OF_THREAD_SAFE; #if !defined(SWIGCSHARP) && !defined(SWIGJAVA) diff --git a/swig/include/java/gdal_java.i b/swig/include/java/gdal_java.i index a357385f37e0..c3ed14adfcbd 100644 --- a/swig/include/java/gdal_java.i +++ b/swig/include/java/gdal_java.i @@ -1312,7 +1312,8 @@ import org.gdal.gdalconst.gdalconstConstants; // Add a Java reference to prevent premature garbage collection and resulting use // of dangling C++ pointer. Intended for methods that return pointers or // references to a member variable. -%typemap(javaout) GDALRasterBandShadow* GetRasterBand, +%typemap(javaout) GDALDatasetShadow* GetThreadSafeDataset, + GDALRasterBandShadow* GetRasterBand, GDALRasterBandShadow* GetOverview, GDALRasterBandShadow* GetMaskBand, GDALColorTableShadow* GetColorTable, diff --git a/swig/include/python/gdal_python.i b/swig/include/python/gdal_python.i index f6fc9feeea18..fe09bada8620 100644 --- a/swig/include/python/gdal_python.i +++ b/swig/include/python/gdal_python.i @@ -1614,6 +1614,16 @@ CPLErr ReadRaster1( double xoff, double yoff, double xsize, double ysize, return get(value) %} +%feature("pythonappend") GetThreadSafeDataset %{ + if val: + val._parent_ds = self + + import weakref + if not hasattr(self, '_child_references'): + self._child_references = weakref.WeakSet() + self._child_references.add(val) +%} + %feature("pythonprepend") Close %{ self._invalidate_children() %} From 80f785d91ae4b45a6f7f73c89898a00d241bfbd8 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Mon, 9 Sep 2024 09:35:01 +0200 Subject: [PATCH 7/9] OGRSpatialReference: add a AssignAndSetThreadSafe() method --- ogr/ogr_spatialref.h | 2 + ogr/ogrspatialreference.cpp | 340 +++++++++++++++++++++++++++++++++++- 2 files changed, 338 insertions(+), 4 deletions(-) diff --git a/ogr/ogr_spatialref.h b/ogr/ogr_spatialref.h index 91d8be2ba575..f1a3b7c91ba0 100644 --- a/ogr/ogr_spatialref.h +++ b/ogr/ogr_spatialref.h @@ -194,6 +194,8 @@ class CPL_DLL OGRSpatialReference OGRSpatialReference &operator=(const OGRSpatialReference &); OGRSpatialReference &operator=(OGRSpatialReference &&); + OGRSpatialReference &AssignAndSetThreadSafe(const OGRSpatialReference &); + int Reference(); int Dereference(); int GetReferenceCount() const; diff --git a/ogr/ogrspatialreference.cpp b/ogr/ogrspatialreference.cpp index 52a5cd487423..fc9bec8b49de 100644 --- a/ogr/ogrspatialreference.cpp +++ b/ogr/ogrspatialreference.cpp @@ -107,6 +107,7 @@ struct OGRSpatialReference::Private std::vector m_wktImportErrors{}; CPLString m_osAreaName{}; + bool m_bIsThreadSafe = false; bool m_bNodesChanged = false; bool m_bNodesWKT2 = false; OGR_SRSNode *m_poRoot = nullptr; @@ -133,7 +134,7 @@ struct OGRSpatialReference::Private std::shared_ptr m_poListener{}; - std::mutex m_mutex{}; + std::recursive_mutex m_mutex{}; OSRAxisMappingStrategy m_axisMappingStrategy = OAMS_AUTHORITY_COMPLIANT; std::vector m_axisMapping{1, 2, 3}; @@ -145,6 +146,11 @@ struct OGRSpatialReference::Private Private(const Private &) = delete; Private &operator=(const Private &) = delete; + void SetThreadSafe() + { + m_bIsThreadSafe = true; + } + void clear(); void setPjCRS(PJ *pj_crsIn, bool doRefreshAxisMapping = true); void setRoot(OGR_SRSNode *poRoot); @@ -172,8 +178,42 @@ struct OGRSpatialReference::Private const char *nullifyTargetKeyIfPossible(const char *pszTargetKey); void refreshAxisMapping(); + + // This structures enables locking during calls to OGRSpatialReference + // public methods. Locking is only needed for instances of + // OGRSpatialReference that have been asked to be thread-safe at + // construction. + // The lock is not just for a single call to OGRSpatialReference::Private, + // but for the series of calls done by a OGRSpatialReference method. + // We need a recursive mutex, because some OGRSpatialReference methods + // may call other ones. + struct OptionalLockGuard + { + Private &m_private; + + explicit OptionalLockGuard(Private *p) : m_private(*p) + { + if (m_private.m_bIsThreadSafe) + m_private.m_mutex.lock(); + } + + ~OptionalLockGuard() + { + if (m_private.m_bIsThreadSafe) + m_private.m_mutex.unlock(); + } + }; + + inline OptionalLockGuard GetOptionalLockGuard() + { + return OptionalLockGuard(this); + } }; +#define TAKE_OPTIONAL_LOCK() \ + auto lock = d->GetOptionalLockGuard(); \ + CPL_IGNORE_RET_VAL(lock) + static OSRAxisMappingStrategy GetDefaultAxisMappingStrategy() { const char *pszDefaultAMS = @@ -989,6 +1029,28 @@ OGRSpatialReference::operator=(OGRSpatialReference &&oSource) return *this; } +/************************************************************************/ +/* AssignAndSetThreadSafe() */ +/************************************************************************/ + +/** Assignment method, with thread-safety. + * + * Same as an assignment operator, but asking also that the *this instance + * becomes thread-safe. + * + * @param oSource SRS to assign to *this + * @return *this + * @since 3.10 + */ + +OGRSpatialReference & +OGRSpatialReference::AssignAndSetThreadSafe(const OGRSpatialReference &oSource) +{ + *this = oSource; + d->SetThreadSafe(); + return *this; +} + /************************************************************************/ /* Reference() */ /************************************************************************/ @@ -1117,6 +1179,8 @@ void OSRRelease(OGRSpatialReferenceH hSRS) OGR_SRSNode *OGRSpatialReference::GetRoot() { + TAKE_OPTIONAL_LOCK(); + if (!d->m_poRoot) { d->refreshRootFromProjObj(); @@ -1126,6 +1190,8 @@ OGR_SRSNode *OGRSpatialReference::GetRoot() const OGR_SRSNode *OGRSpatialReference::GetRoot() const { + TAKE_OPTIONAL_LOCK(); + if (!d->m_poRoot) { d->refreshRootFromProjObj(); @@ -1320,6 +1386,8 @@ const char *CPL_STDCALL OSRGetAttrValue(OGRSpatialReferenceH hSRS, const char *OGRSpatialReference::GetName() const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); if (!d->m_pj_crs) return nullptr; @@ -1378,6 +1446,8 @@ OGRSpatialReference *OGRSpatialReference::Clone() const { OGRSpatialReference *poNewRef = new OGRSpatialReference(); + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); if (d->m_pj_crs != nullptr) poNewRef->d->setPjCRS(proj_clone(d->getPROJContext(), d->m_pj_crs)); @@ -1613,7 +1683,7 @@ OGRErr OGRSpatialReference::exportToWkt(char **ppszResult, // In the past calling this method was thread-safe, even if we never // guaranteed it. Now proj_as_wkt() will cache the result internally, // so this is no longer thread-safe. - std::lock_guard oLock(d->m_mutex); + std::lock_guard oLock(d->m_mutex); d->refreshProjObj(); if (!d->m_pj_crs) @@ -1949,6 +2019,8 @@ OGRErr OSRExportToWktEx(OGRSpatialReferenceH hSRS, char **ppszReturn, OGRErr OGRSpatialReference::exportToPROJJSON( char **ppszResult, CPL_UNUSED const char *const *papszOptions) const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); if (!d->m_pj_crs) { @@ -2114,6 +2186,8 @@ OGRErr OGRSpatialReference::importFromWkt(const char **ppszInput, CSLConstList papszOptions) { + TAKE_OPTIONAL_LOCK(); + if (!ppszInput || !*ppszInput) return OGRERR_FAILURE; @@ -2297,6 +2371,8 @@ OGRErr OGRSpatialReference::importFromWkt(const char *pszInput) OGRErr OGRSpatialReference::Validate() const { + TAKE_OPTIONAL_LOCK(); + for (const auto &str : d->m_wktImportErrors) { CPLDebug("OGRSpatialReference::Validate", "%s", str.c_str()); @@ -2380,6 +2456,8 @@ OGRErr OGRSpatialReference::SetNode(const char *pszNodePath, const char *pszNewNodeValue) { + TAKE_OPTIONAL_LOCK(); + char **papszPathTokens = CSLTokenizeStringComplex(pszNodePath, "|", TRUE, FALSE); @@ -2517,6 +2595,8 @@ OGRErr OGRSpatialReference::SetAngularUnits(const char *pszUnitsName, double dfInRadians) { + TAKE_OPTIONAL_LOCK(); + d->bNormInfoSet = FALSE; d->refreshProjObj(); @@ -2582,6 +2662,8 @@ OGRErr OSRSetAngularUnits(OGRSpatialReferenceH hSRS, const char *pszUnits, double OGRSpatialReference::GetAngularUnits(const char **ppszName) const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); if (!d->m_osAngularUnits.empty()) @@ -2720,6 +2802,8 @@ OGRErr OGRSpatialReference::SetLinearUnitsAndUpdateParameters( const char *pszUnitCode) { + TAKE_OPTIONAL_LOCK(); + if (dfInMeters <= 0.0) return OGRERR_FAILURE; @@ -2855,6 +2939,8 @@ OGRErr OGRSpatialReference::SetTargetLinearUnits(const char *pszTargetKey, const char *pszUnitCode) { + TAKE_OPTIONAL_LOCK(); + if (dfInMeters <= 0.0) return OGRERR_FAILURE; @@ -3044,6 +3130,8 @@ double OGRSpatialReference::GetTargetLinearUnits(const char *pszTargetKey, const char **ppszName) const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); pszTargetKey = d->nullifyTargetKeyIfPossible(pszTargetKey); @@ -3271,6 +3359,8 @@ double OSRGetTargetLinearUnits(OGRSpatialReferenceH hSRS, double OGRSpatialReference::GetPrimeMeridian(const char **ppszName) const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); if (!d->m_osPrimeMeridianName.empty()) @@ -3408,6 +3498,8 @@ OGRErr OGRSpatialReference::SetGeogCS( double dfConvertToRadians) { + TAKE_OPTIONAL_LOCK(); + d->bNormInfoSet = FALSE; d->m_osAngularUnits.clear(); d->m_dfAngularUnitToRadian = 0.0; @@ -3516,6 +3608,8 @@ OGRErr OSRSetGeogCS(OGRSpatialReferenceH hSRS, const char *pszGeogName, OGRErr OGRSpatialReference::SetWellKnownGeogCS(const char *pszName) { + TAKE_OPTIONAL_LOCK(); + /* -------------------------------------------------------------------- */ /* Check for EPSG authority numbers. */ /* -------------------------------------------------------------------- */ @@ -3658,6 +3752,8 @@ OGRErr OSRSetWellKnownGeogCS(OGRSpatialReferenceH hSRS, const char *pszName) OGRErr OGRSpatialReference::CopyGeogCSFrom(const OGRSpatialReference *poSrcSRS) { + TAKE_OPTIONAL_LOCK(); + d->bNormInfoSet = FALSE; d->m_osAngularUnits.clear(); d->m_dfAngularUnitToRadian = 0.0; @@ -3907,6 +4003,8 @@ OGRErr OGRSpatialReference::SetFromUserInput(const char *pszDefinition) OGRErr OGRSpatialReference::SetFromUserInput(const char *pszDefinition, CSLConstList papszOptions) { + TAKE_OPTIONAL_LOCK(); + // Skip leading white space while (isspace(static_cast(*pszDefinition))) pszDefinition++; @@ -4274,6 +4372,8 @@ OGRErr OSRSetFromUserInputEx(OGRSpatialReferenceH hSRS, const char *pszDef, OGRErr OGRSpatialReference::importFromUrl(const char *pszUrl) { + TAKE_OPTIONAL_LOCK(); + if (!STARTS_WITH_CI(pszUrl, "http://") && !STARTS_WITH_CI(pszUrl, "https://")) { @@ -4484,6 +4584,8 @@ OGRErr OGRSpatialReference::importFromURNPart(const char *pszAuthority, OGRErr OGRSpatialReference::importFromURN(const char *pszURN) { + TAKE_OPTIONAL_LOCK(); + #if PROJ_AT_LEAST_VERSION(8, 1, 0) // PROJ 8.2.0 has support for IAU codes now. @@ -4672,6 +4774,8 @@ OGRErr OGRSpatialReference::importFromURN(const char *pszURN) OGRErr OGRSpatialReference::importFromCRSURL(const char *pszURL) { + TAKE_OPTIONAL_LOCK(); + #if PROJ_AT_LEAST_VERSION(8, 1, 0) if (strlen(pszURL) >= 10000) { @@ -4853,6 +4957,8 @@ OGRErr OGRSpatialReference::importFromCRSURL(const char *pszURL) OGRErr OGRSpatialReference::importFromWMSAUTO(const char *pszDefinition) { + TAKE_OPTIONAL_LOCK(); + #if PROJ_AT_LEAST_VERSION(8, 1, 0) if (strlen(pszDefinition) >= 10000) { @@ -5010,6 +5116,8 @@ OGRErr OGRSpatialReference::importFromWMSAUTO(const char *pszDefinition) double OGRSpatialReference::GetSemiMajor(OGRErr *pnErr) const { + TAKE_OPTIONAL_LOCK(); + if (pnErr != nullptr) *pnErr = OGRERR_FAILURE; @@ -5071,6 +5179,8 @@ double OSRGetSemiMajor(OGRSpatialReferenceH hSRS, OGRErr *pnErr) double OGRSpatialReference::GetInvFlattening(OGRErr *pnErr) const { + TAKE_OPTIONAL_LOCK(); + if (pnErr != nullptr) *pnErr = OGRERR_FAILURE; @@ -5230,6 +5340,8 @@ double OSRGetSemiMinor(OGRSpatialReferenceH hSRS, OGRErr *pnErr) OGRErr OGRSpatialReference::SetLocalCS(const char *pszName) { + TAKE_OPTIONAL_LOCK(); + if (d->m_pjType == PJ_TYPE_UNKNOWN || d->m_pjType == PJ_TYPE_ENGINEERING_CRS) { @@ -5288,6 +5400,8 @@ OGRErr OSRSetLocalCS(OGRSpatialReferenceH hSRS, const char *pszName) OGRErr OGRSpatialReference::SetGeocCS(const char *pszName) { + TAKE_OPTIONAL_LOCK(); + OGRErr eErr = OGRERR_NONE; d->refreshProjObj(); d->demoteFromBoundCRS(); @@ -5391,6 +5505,8 @@ OGRErr OGRSpatialReference::SetVertCS(const char *pszVertCSName, int nVertDatumType) { + TAKE_OPTIONAL_LOCK(); + CPL_IGNORE_RET_VAL(nVertDatumType); d->refreshProjObj(); @@ -5463,6 +5579,8 @@ OGRErr OGRSpatialReference::SetCompoundCS(const char *pszName, const OGRSpatialReference *poVertSRS) { + TAKE_OPTIONAL_LOCK(); + /* -------------------------------------------------------------------- */ /* Verify these are legal horizontal and vertical coordinate */ /* systems. */ @@ -5537,6 +5655,8 @@ OGRErr OSRSetCompoundCS(OGRSpatialReferenceH hSRS, const char *pszName, OGRErr OGRSpatialReference::SetProjCS(const char *pszName) { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); d->demoteFromBoundCRS(); if (d->m_pjType == PJ_TYPE_PROJECTED_CRS) @@ -5597,6 +5717,8 @@ OGRErr OSRSetProjCS(OGRSpatialReferenceH hSRS, const char *pszName) OGRErr OGRSpatialReference::SetProjection(const char *pszProjection) { + TAKE_OPTIONAL_LOCK(); + OGR_SRSNode *poGeogCS = nullptr; if (GetRoot() != nullptr && EQUAL(d->m_poRoot->GetValue(), "GEOGCS")) @@ -5666,6 +5788,8 @@ OGRSpatialReference::GetWKT2ProjectionMethod(const char **ppszMethodName, const char **ppszMethodAuthName, const char **ppszMethodCode) const { + TAKE_OPTIONAL_LOCK(); + auto conv = proj_crs_get_coordoperation(d->getPROJContext(), d->m_pj_crs); if (!conv) return OGRERR_FAILURE; @@ -5717,6 +5841,8 @@ OGRErr OGRSpatialReference::SetProjParm(const char *pszParamName, double dfValue) { + TAKE_OPTIONAL_LOCK(); + OGR_SRSNode *poPROJCS = GetAttrNode("PROJCS"); if (poPROJCS == nullptr) @@ -5789,6 +5915,8 @@ int OGRSpatialReference::FindProjParm(const char *pszParameter, const OGR_SRSNode *poPROJCS) const { + TAKE_OPTIONAL_LOCK(); + if (poPROJCS == nullptr) poPROJCS = GetAttrNode("PROJCS"); @@ -5883,6 +6011,8 @@ double OGRSpatialReference::GetProjParm(const char *pszName, OGRErr *pnErr) const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); GetRoot(); // force update of d->m_bNodesWKT2 @@ -5969,6 +6099,8 @@ double OGRSpatialReference::GetNormProjParm(const char *pszName, OGRErr *pnErr) const { + TAKE_OPTIONAL_LOCK(); + GetNormInfo(); OGRErr nError = OGRERR_NONE; @@ -6032,6 +6164,8 @@ double OSRGetNormProjParm(OGRSpatialReferenceH hSRS, const char *pszName, OGRErr OGRSpatialReference::SetNormProjParm(const char *pszName, double dfValue) { + TAKE_OPTIONAL_LOCK(); + GetNormInfo(); if (d->dfToDegrees != 0.0 && @@ -6074,6 +6208,8 @@ OGRErr OGRSpatialReference::SetTM(double dfCenterLat, double dfCenterLong, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_transverse_mercator( d->getPROJContext(), dfCenterLat, dfCenterLong, dfScale, @@ -6106,6 +6242,8 @@ OGRErr OGRSpatialReference::SetTMVariant(const char *pszVariantName, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + SetProjection(pszVariantName); SetNormProjParm(SRS_PP_LATITUDE_OF_ORIGIN, dfCenterLat); SetNormProjParm(SRS_PP_CENTRAL_MERIDIAN, dfCenterLong); @@ -6141,6 +6279,8 @@ OGRErr OGRSpatialReference::SetTMSO(double dfCenterLat, double dfCenterLong, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + auto conv = proj_create_conversion_transverse_mercator_south_oriented( d->getPROJContext(), dfCenterLat, dfCenterLong, dfScale, dfFalseEasting, dfFalseNorthing, nullptr, 0.0, nullptr, 0.0); @@ -6194,6 +6334,8 @@ OGRErr OGRSpatialReference::SetTPED(double dfLat1, double dfLong1, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_two_point_equidistant( d->getPROJContext(), dfLat1, dfLong1, dfLat2, dfLong2, @@ -6224,6 +6366,8 @@ OGRErr OGRSpatialReference::SetTMG(double dfCenterLat, double dfCenterLong, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_tunisia_mapping_grid( d->getPROJContext(), dfCenterLat, dfCenterLong, dfFalseEasting, @@ -6255,6 +6399,8 @@ OGRErr OGRSpatialReference::SetACEA(double dfStdP1, double dfStdP2, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + // Note different order of parameters. The one in PROJ is conformant with // EPSG return d->replaceConversionAndUnref( @@ -6286,6 +6432,8 @@ OGRErr OGRSpatialReference::SetAE(double dfCenterLat, double dfCenterLong, double dfFalseEasting, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_azimuthal_equidistant( d->getPROJContext(), dfCenterLat, dfCenterLong, dfFalseEasting, @@ -6316,6 +6464,8 @@ OGRErr OGRSpatialReference::SetBonne(double dfStdP1, double dfCentralMeridian, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref(proj_create_conversion_bonne( d->getPROJContext(), dfStdP1, dfCentralMeridian, dfFalseEasting, dfFalseNorthing, nullptr, 0.0, nullptr, 0.0)); @@ -6345,6 +6495,8 @@ OGRErr OGRSpatialReference::SetCEA(double dfStdP1, double dfCentralMeridian, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_lambert_cylindrical_equal_area( d->getPROJContext(), dfStdP1, dfCentralMeridian, dfFalseEasting, @@ -6374,6 +6526,8 @@ OGRErr OGRSpatialReference::SetCS(double dfCenterLat, double dfCenterLong, double dfFalseEasting, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref(proj_create_conversion_cassini_soldner( d->getPROJContext(), dfCenterLat, dfCenterLong, dfFalseEasting, dfFalseNorthing, nullptr, 0.0, nullptr, 0.0)); @@ -6403,6 +6557,8 @@ OGRErr OGRSpatialReference::SetEC(double dfStdP1, double dfStdP2, double dfFalseEasting, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + // Note: different order of arguments return d->replaceConversionAndUnref( proj_create_conversion_equidistant_conic( @@ -6435,6 +6591,8 @@ OGRErr OGRSpatialReference::SetEckert(int nVariation, // 1-6. double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + PJ *conv; if (nVariation == 1) { @@ -6563,6 +6721,8 @@ OGRErr OGRSpatialReference::SetEquirectangular(double dfCenterLat, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + if (dfCenterLat == 0.0) { return d->replaceConversionAndUnref( @@ -6608,6 +6768,8 @@ OGRErr OGRSpatialReference::SetEquirectangular2(double dfCenterLat, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + if (dfCenterLat == 0.0) { return d->replaceConversionAndUnref( @@ -6678,6 +6840,8 @@ OGRErr OGRSpatialReference::SetGH(double dfCentralMeridian, double dfFalseEasting, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref(proj_create_conversion_goode_homolosine( d->getPROJContext(), dfCentralMeridian, dfFalseEasting, dfFalseNorthing, nullptr, 0.0, nullptr, 0.0)); @@ -6704,6 +6868,8 @@ OGRErr OSRSetGH(OGRSpatialReferenceH hSRS, double dfCentralMeridian, OGRErr OGRSpatialReference::SetIGH() { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_interrupted_goode_homolosine( d->getPROJContext(), 0.0, 0.0, 0.0, nullptr, 0.0, nullptr, 0.0)); @@ -6731,6 +6897,8 @@ OGRErr OGRSpatialReference::SetGEOS(double dfCentralMeridian, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_geostationary_satellite_sweep_y( d->getPROJContext(), dfCentralMeridian, dfSatelliteHeight, @@ -6763,6 +6931,8 @@ OGRErr OGRSpatialReference::SetGaussSchreiberTMercator(double dfCenterLat, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_gauss_schreiber_transverse_mercator( d->getPROJContext(), dfCenterLat, dfCenterLong, dfScale, @@ -6794,6 +6964,8 @@ OGRErr OGRSpatialReference::SetGnomonic(double dfCenterLat, double dfCenterLong, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref(proj_create_conversion_gnomonic( d->getPROJContext(), dfCenterLat, dfCenterLong, dfFalseEasting, dfFalseNorthing, nullptr, 0.0, nullptr, 0.0)); @@ -6845,6 +7017,8 @@ OGRErr OGRSpatialReference::SetHOMAC(double dfCenterLat, double dfCenterLong, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_hotine_oblique_mercator_variant_b( d->getPROJContext(), dfCenterLat, dfCenterLong, dfAzimuth, @@ -6904,6 +7078,8 @@ OGRErr OGRSpatialReference::SetHOM(double dfCenterLat, double dfCenterLong, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_hotine_oblique_mercator_variant_a( d->getPROJContext(), dfCenterLat, dfCenterLong, dfAzimuth, @@ -6960,6 +7136,8 @@ OGRErr OGRSpatialReference::SetHOM2PNO(double dfCenterLat, double dfLat1, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_hotine_oblique_mercator_two_point_natural_origin( d->getPROJContext(), dfCenterLat, dfLat1, dfLong1, dfLat2, dfLong2, @@ -7013,6 +7191,8 @@ OGRErr OGRSpatialReference::SetLOM(double dfCenterLat, double dfCenterLong, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_laborde_oblique_mercator( d->getPROJContext(), dfCenterLat, dfCenterLong, dfAzimuth, dfScale, @@ -7029,6 +7209,8 @@ OGRErr OGRSpatialReference::SetIWMPolyconic(double dfLat1, double dfLat2, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_international_map_world_polyconic( d->getPROJContext(), dfCenterLong, dfLat1, dfLat2, dfFalseEasting, @@ -7066,6 +7248,8 @@ OGRErr OGRSpatialReference::SetKrovak(double dfCenterLat, double dfCenterLong, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_krovak_north_oriented( d->getPROJContext(), dfCenterLat, dfCenterLong, dfAzimuth, @@ -7099,6 +7283,8 @@ OGRErr OGRSpatialReference::SetLAEA(double dfCenterLat, double dfCenterLong, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + auto conv = proj_create_conversion_lambert_azimuthal_equal_area( d->getPROJContext(), dfCenterLat, dfCenterLong, dfFalseEasting, dfFalseNorthing, nullptr, 0.0, nullptr, 0.0); @@ -7157,6 +7343,8 @@ OGRErr OGRSpatialReference::SetLCC(double dfStdP1, double dfStdP2, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_lambert_conic_conformal_2sp( d->getPROJContext(), dfCenterLat, dfCenterLong, dfStdP1, dfStdP2, @@ -7187,6 +7375,8 @@ OGRErr OGRSpatialReference::SetLCC1SP(double dfCenterLat, double dfCenterLong, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_lambert_conic_conformal_1sp( d->getPROJContext(), dfCenterLat, dfCenterLong, dfScale, @@ -7218,6 +7408,8 @@ OGRErr OGRSpatialReference::SetLCCB(double dfStdP1, double dfStdP2, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_lambert_conic_conformal_2sp_belgium( d->getPROJContext(), dfCenterLat, dfCenterLong, dfStdP1, dfStdP2, @@ -7247,6 +7439,8 @@ OGRErr OGRSpatialReference::SetMC(double dfCenterLat, double dfCenterLong, double dfFalseEasting, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + (void)dfCenterLat; // ignored return d->replaceConversionAndUnref( @@ -7279,6 +7473,8 @@ OGRErr OGRSpatialReference::SetMercator(double dfCenterLat, double dfCenterLong, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + if (dfCenterLat != 0.0 && dfScale == 1.0) { // Not sure this is correct, but this is how it has been used @@ -7327,6 +7523,8 @@ OGRErr OGRSpatialReference::SetMercator2SP(double dfStdP1, double dfCenterLat, dfFalseNorthing, nullptr, 0, nullptr, 0)); } + TAKE_OPTIONAL_LOCK(); + SetProjection(SRS_PT_MERCATOR_2SP); SetNormProjParm(SRS_PP_STANDARD_PARALLEL_1, dfStdP1); @@ -7362,6 +7560,8 @@ OGRErr OGRSpatialReference::SetMollweide(double dfCentralMeridian, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref(proj_create_conversion_mollweide( d->getPROJContext(), dfCentralMeridian, dfFalseEasting, dfFalseNorthing, nullptr, 0, nullptr, 0)); @@ -7390,6 +7590,8 @@ OGRErr OGRSpatialReference::SetNZMG(double dfCenterLat, double dfCenterLong, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_new_zealand_mapping_grid( d->getPROJContext(), dfCenterLat, dfCenterLong, dfFalseEasting, @@ -7420,6 +7622,8 @@ OGRErr OGRSpatialReference::SetOS(double dfOriginLat, double dfCMeridian, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_oblique_stereographic( d->getPROJContext(), dfOriginLat, dfCMeridian, dfScale, @@ -7451,6 +7655,8 @@ OGRErr OGRSpatialReference::SetOrthographic(double dfCenterLat, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref(proj_create_conversion_orthographic( d->getPROJContext(), dfCenterLat, dfCenterLong, dfFalseEasting, dfFalseNorthing, nullptr, 0, nullptr, 0)); @@ -7481,6 +7687,8 @@ OGRErr OGRSpatialReference::SetPolyconic(double dfCenterLat, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + // note: it seems that by some definitions this should include a // scale_factor parameter. return d->replaceConversionAndUnref( @@ -7522,6 +7730,8 @@ OGRErr OGRSpatialReference::SetPS(double dfCenterLat, double dfCenterLong, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + PJ *conv; if (dfScale == 1.0 && std::abs(std::abs(dfCenterLat) - 90) > 1e-8) { @@ -7586,6 +7796,8 @@ OGRErr OGRSpatialReference::SetRobinson(double dfCenterLong, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref(proj_create_conversion_robinson( d->getPROJContext(), dfCenterLong, dfFalseEasting, dfFalseNorthing, nullptr, 0, nullptr, 0)); @@ -7614,6 +7826,8 @@ OGRErr OGRSpatialReference::SetSinusoidal(double dfCenterLong, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref(proj_create_conversion_sinusoidal( d->getPROJContext(), dfCenterLong, dfFalseEasting, dfFalseNorthing, nullptr, 0, nullptr, 0)); @@ -7643,6 +7857,8 @@ OGRErr OGRSpatialReference::SetStereographic(double dfOriginLat, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref(proj_create_conversion_stereographic( d->getPROJContext(), dfOriginLat, dfCMeridian, dfScale, dfFalseEasting, dfFalseNorthing, nullptr, 0, nullptr, 0)); @@ -7679,6 +7895,8 @@ OGRErr OGRSpatialReference::SetSOC(double dfLatitudeOfOrigin, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_hotine_oblique_mercator_variant_b( d->getPROJContext(), dfLatitudeOfOrigin, dfCentralMeridian, 90.0, @@ -7718,6 +7936,8 @@ OGRErr OGRSpatialReference::SetVDG(double dfCMeridian, double dfFalseEasting, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref(proj_create_conversion_van_der_grinten( d->getPROJContext(), dfCMeridian, dfFalseEasting, dfFalseNorthing, nullptr, 0, nullptr, 0)); @@ -7762,6 +7982,8 @@ OGRErr OSRSetVDG(OGRSpatialReferenceH hSRS, double dfCentralMeridian, OGRErr OGRSpatialReference::SetUTM(int nZone, int bNorth) { + TAKE_OPTIONAL_LOCK(); + if (nZone < 0 || nZone > 60) { CPLError(CE_Failure, CPLE_AppDefined, "Invalid zone: %d", nZone); @@ -7813,6 +8035,8 @@ OGRErr OSRSetUTM(OGRSpatialReferenceH hSRS, int nZone, int bNorth) int OGRSpatialReference::GetUTMZone(int *pbNorth) const { + TAKE_OPTIONAL_LOCK(); + if (IsProjected() && GetAxesCount() == 3) { OGRSpatialReference *poSRSTmp = Clone(); @@ -7883,6 +8107,8 @@ OGRErr OGRSpatialReference::SetWagner(int nVariation, // 1--7. double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + PJ *conv; if (nVariation == 1) { @@ -7957,6 +8183,8 @@ OGRErr OSRSetWagner(OGRSpatialReferenceH hSRS, int nVariation, OGRErr OGRSpatialReference::SetQSC(double dfCenterLat, double dfCenterLong) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_quadrilateralized_spherical_cube( d->getPROJContext(), dfCenterLat, dfCenterLong, 0.0, 0.0, nullptr, @@ -7984,6 +8212,8 @@ OGRErr OGRSpatialReference::SetSCH(double dfPegLat, double dfPegLong, double dfPegHeading, double dfPegHgt) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_spherical_cross_track_height( d->getPROJContext(), dfPegLat, dfPegLong, dfPegHeading, dfPegHgt, @@ -8011,6 +8241,8 @@ OGRErr OGRSpatialReference::SetVerticalPerspective( double dfTopoOriginLat, double dfTopoOriginLon, double dfTopoOriginHeight, double dfViewPointHeight, double dfFalseEasting, double dfFalseNorthing) { + TAKE_OPTIONAL_LOCK(); + return d->replaceConversionAndUnref( proj_create_conversion_vertical_perspective( d->getPROJContext(), dfTopoOriginLat, dfTopoOriginLon, @@ -8044,6 +8276,8 @@ OGRErr OGRSpatialReference::SetDerivedGeogCRSWithPoleRotationGRIBConvention( const char *pszCRSName, double dfSouthPoleLat, double dfSouthPoleLon, double dfAxisRotation) { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); if (!d->m_pj_crs) return OGRERR_FAILURE; @@ -8068,6 +8302,8 @@ OGRErr OGRSpatialReference::SetDerivedGeogCRSWithPoleRotationNetCDFCFConvention( const char *pszCRSName, double dfGridNorthPoleLat, double dfGridNorthPoleLon, double dfNorthPoleGridLon) { + TAKE_OPTIONAL_LOCK(); + #if PROJ_VERSION_MAJOR > 8 || \ (PROJ_VERSION_MAJOR == 8 && PROJ_VERSION_MINOR >= 2) d->refreshProjObj(); @@ -8124,6 +8360,8 @@ OGRErr OGRSpatialReference::SetAuthority(const char *pszTargetKey, const char *pszAuthority, int nCode) { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); pszTargetKey = d->nullifyTargetKeyIfPossible(pszTargetKey); @@ -8254,6 +8492,8 @@ const char * OGRSpatialReference::GetAuthorityCode(const char *pszTargetKey) const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); const char *pszInputTargetKey = pszTargetKey; pszTargetKey = d->nullifyTargetKeyIfPossible(pszTargetKey); @@ -8388,6 +8628,8 @@ const char * OGRSpatialReference::GetAuthorityName(const char *pszTargetKey) const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); const char *pszInputTargetKey = pszTargetKey; pszTargetKey = d->nullifyTargetKeyIfPossible(pszTargetKey); @@ -8507,7 +8749,7 @@ const char *OSRGetAuthorityName(OGRSpatialReferenceH hSRS, * a compound CRS whose horizontal and vertical parts have a top-level * identifier. * - * @return a string to free with CPLFree(), or nulptr when no result can be + * @return a string to free with CPLFree(), or nullptr when no result can be * generated * * @since GDAL 3.5 @@ -8516,6 +8758,8 @@ const char *OSRGetAuthorityName(OGRSpatialReferenceH hSRS, char *OGRSpatialReference::GetOGCURN() const { + TAKE_OPTIONAL_LOCK(); + const char *pszAuthName = GetAuthorityName(nullptr); const char *pszAuthCode = GetAuthorityCode(nullptr); if (pszAuthName && pszAuthCode) @@ -8565,6 +8809,8 @@ char *OGRSpatialReference::GetOGCURN() const OGRErr OGRSpatialReference::StripVertical() { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); d->demoteFromBoundCRS(); if (!d->m_pj_crs || d->m_pjType != PJ_TYPE_COMPOUND_CRS) @@ -8668,6 +8914,8 @@ bool OGRSpatialReference::StripTOWGS84IfKnownDatumAndAllowed() bool OGRSpatialReference::StripTOWGS84IfKnownDatum() { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); if (!d->m_pj_crs || d->m_pjType != PJ_TYPE_BOUND_CRS) { @@ -8758,6 +9006,8 @@ bool OGRSpatialReference::StripTOWGS84IfKnownDatum() int OGRSpatialReference::IsCompound() const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); d->demoteFromBoundCRS(); bool isCompound = d->m_pjType == PJ_TYPE_COMPOUND_CRS; @@ -8799,6 +9049,8 @@ int OSRIsCompound(OGRSpatialReferenceH hSRS) int OGRSpatialReference::IsProjected() const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); d->demoteFromBoundCRS(); bool isProjected = d->m_pjType == PJ_TYPE_PROJECTED_CRS; @@ -8860,6 +9112,8 @@ int OSRIsProjected(OGRSpatialReferenceH hSRS) int OGRSpatialReference::IsGeocentric() const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); d->demoteFromBoundCRS(); bool isGeocentric = d->m_pjType == PJ_TYPE_GEOCENTRIC_CRS; @@ -8895,6 +9149,8 @@ int OSRIsGeocentric(OGRSpatialReferenceH hSRS) bool OGRSpatialReference::IsEmpty() const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); return d->m_pj_crs == nullptr; } @@ -8916,6 +9172,8 @@ bool OGRSpatialReference::IsEmpty() const int OGRSpatialReference::IsGeographic() const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); d->demoteFromBoundCRS(); bool isGeog = d->m_pjType == PJ_TYPE_GEOGRAPHIC_2D_CRS || @@ -8979,6 +9237,8 @@ int OSRIsGeographic(OGRSpatialReferenceH hSRS) int OGRSpatialReference::IsDerivedGeographic() const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); d->demoteFromBoundCRS(); const bool isGeog = d->m_pjType == PJ_TYPE_GEOGRAPHIC_2D_CRS || @@ -9022,6 +9282,7 @@ int OGRSpatialReference::IsDerivedProjected() const { #if PROJ_AT_LEAST_VERSION(9, 2, 0) + TAKE_OPTIONAL_LOCK(); d->refreshProjObj(); d->demoteFromBoundCRS(); const bool isDerivedProjected = @@ -9067,6 +9328,7 @@ int OSRIsDerivedProjected(OGRSpatialReferenceH hSRS) int OGRSpatialReference::IsLocal() const { + TAKE_OPTIONAL_LOCK(); d->refreshProjObj(); return d->m_pjType == PJ_TYPE_ENGINEERING_CRS; } @@ -9106,6 +9368,7 @@ int OSRIsLocal(OGRSpatialReferenceH hSRS) int OGRSpatialReference::IsVertical() const { + TAKE_OPTIONAL_LOCK(); d->refreshProjObj(); d->demoteFromBoundCRS(); bool isVertical = d->m_pjType == PJ_TYPE_VERTICAL_CRS; @@ -9173,6 +9436,7 @@ int OSRIsVertical(OGRSpatialReferenceH hSRS) bool OGRSpatialReference::IsDynamic() const { + TAKE_OPTIONAL_LOCK(); bool isDynamic = false; d->refreshProjObj(); d->demoteFromBoundCRS(); @@ -9282,6 +9546,7 @@ bool OGRSpatialReference::HasPointMotionOperation() const { #if PROJ_VERSION_MAJOR > 9 || \ (PROJ_VERSION_MAJOR == 9 && PROJ_VERSION_MINOR >= 4) + TAKE_OPTIONAL_LOCK(); d->refreshProjObj(); d->demoteFromBoundCRS(); auto ctxt = d->getPROJContext(); @@ -9330,6 +9595,7 @@ int OSRHasPointMotionOperation(OGRSpatialReferenceH hSRS) OGRSpatialReference *OGRSpatialReference::CloneGeogCS() const { + TAKE_OPTIONAL_LOCK(); d->refreshProjObj(); if (d->m_pj_crs) { @@ -9447,6 +9713,8 @@ int OGRSpatialReference::IsSameGeogCS(const OGRSpatialReference *poOther, const char *const *papszOptions) const { + TAKE_OPTIONAL_LOCK(); + CPL_IGNORE_RET_VAL(papszOptions); d->refreshProjObj(); @@ -9514,6 +9782,8 @@ int OSRIsSameGeogCS(OGRSpatialReferenceH hSRS1, OGRSpatialReferenceH hSRS2) int OGRSpatialReference::IsSameVertCS(const OGRSpatialReference *poOther) const { + TAKE_OPTIONAL_LOCK(); + /* -------------------------------------------------------------------- */ /* Does the datum name match? */ /* -------------------------------------------------------------------- */ @@ -9599,6 +9869,8 @@ int OGRSpatialReference::IsSame(const OGRSpatialReference *poOtherSRS, const char *const *papszOptions) const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); poOtherSRS->d->refreshProjObj(); if (!d->m_pj_crs || !poOtherSRS->d->m_pj_crs) @@ -9718,6 +9990,8 @@ OGRSpatialReference *OGRSpatialReference::convertToOtherProjection( const char *pszTargetProjection, CPL_UNUSED const char *const *papszOptions) const { + TAKE_OPTIONAL_LOCK(); + if (pszTargetProjection == nullptr) return nullptr; int new_code; @@ -9923,6 +10197,8 @@ OGRSpatialReference::FindBestMatch(int nMinimumMatchConfidence, const char *pszPreferredAuthority, CSLConstList papszOptions) const { + TAKE_OPTIONAL_LOCK(); + CPL_IGNORE_RET_VAL(papszOptions); // ignored for now. if (nMinimumMatchConfidence == 0) @@ -10049,6 +10325,8 @@ OGRErr OGRSpatialReference::SetTOWGS84(double dfDX, double dfDY, double dfDZ, double dfPPM) { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); if (d->m_pj_crs == nullptr) { @@ -10220,6 +10498,8 @@ OGRErr OSRSetTOWGS84(OGRSpatialReferenceH hSRS, double dfDX, double dfDY, OGRErr OGRSpatialReference::GetTOWGS84(double *padfCoeff, int nCoeffCount) const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); if (d->m_pjType != PJ_TYPE_BOUND_CRS) return OGRERR_FAILURE; @@ -10261,6 +10541,7 @@ OGRErr OSRGetTOWGS84(OGRSpatialReferenceH hSRS, double *padfCoeff, * @return TRUE or FALSE */ +/* static */ int OGRSpatialReference::IsAngularParameter(const char *pszParameterName) { @@ -10285,6 +10566,7 @@ int OGRSpatialReference::IsAngularParameter(const char *pszParameterName) * @return TRUE or FALSE */ +/* static */ int OGRSpatialReference::IsLongitudeParameter(const char *pszParameterName) { @@ -10304,6 +10586,8 @@ int OGRSpatialReference::IsLongitudeParameter(const char *pszParameterName) * * @return TRUE or FALSE */ + +/* static */ int OGRSpatialReference::IsLinearParameter(const char *pszParameterName) { @@ -10325,6 +10609,8 @@ int OGRSpatialReference::IsLinearParameter(const char *pszParameterName) void OGRSpatialReference::GetNormInfo() const { + TAKE_OPTIONAL_LOCK(); + if (d->bNormInfoSet) return; @@ -10362,6 +10648,8 @@ const char *OGRSpatialReference::GetExtension(const char *pszTargetKey, const char *pszDefault) const { + TAKE_OPTIONAL_LOCK(); + /* -------------------------------------------------------------------- */ /* Find the target node. */ /* -------------------------------------------------------------------- */ @@ -10410,6 +10698,8 @@ OGRErr OGRSpatialReference::SetExtension(const char *pszTargetKey, const char *pszValue) { + TAKE_OPTIONAL_LOCK(); + /* -------------------------------------------------------------------- */ /* Find the target node. */ /* -------------------------------------------------------------------- */ @@ -10491,6 +10781,8 @@ void OSRCleanup(void) */ int OGRSpatialReference::GetAxesCount() const { + TAKE_OPTIONAL_LOCK(); + int axisCount = 0; d->refreshProjObj(); if (d->m_pj_crs == nullptr) @@ -10588,6 +10880,8 @@ const char *OGRSpatialReference::GetAxis(const char *pszTargetKey, int iAxis, double *pdfConvUnit) const { + TAKE_OPTIONAL_LOCK(); + if (peOrientation != nullptr) *peOrientation = OAO_Other; if (pdfConvUnit != nullptr) @@ -10852,6 +11146,8 @@ OGRErr OGRSpatialReference::SetAxes(const char *pszTargetKey, OGRAxisOrientation eYAxisOrientation) { + TAKE_OPTIONAL_LOCK(); + /* -------------------------------------------------------------------- */ /* Find the target node. */ /* -------------------------------------------------------------------- */ @@ -11177,6 +11473,8 @@ OGRErr OSRImportFromProj4(OGRSpatialReferenceH hSRS, const char *pszProj4) OGRErr OGRSpatialReference::importFromProj4(const char *pszProj4) { + TAKE_OPTIONAL_LOCK(); + if (strlen(pszProj4) >= 10000) { CPLError(CE_Failure, CPLE_AppDefined, "Too long PROJ string"); @@ -11285,7 +11583,7 @@ OGRErr OGRSpatialReference::exportToProj4(char **ppszProj4) const // In the past calling this method was thread-safe, even if we never // guaranteed it. Now proj_as_proj_string() will cache the result // internally, so this is no longer thread-safe. - std::lock_guard oLock(d->m_mutex); + std::lock_guard oLock(d->m_mutex); d->refreshProjObj(); if (d->m_pj_crs == nullptr || d->m_pjType == PJ_TYPE_ENGINEERING_CRS) @@ -11380,6 +11678,8 @@ OGRErr OGRSpatialReference::exportToProj4(char **ppszProj4) const OGRErr OGRSpatialReference::morphToESRI() { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); d->setMorphToESRI(true); @@ -11430,6 +11730,8 @@ OGRErr OSRMorphToESRI(OGRSpatialReferenceH hSRS) OGRErr OGRSpatialReference::morphFromESRI() { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); d->setMorphToESRI(false); @@ -11489,6 +11791,8 @@ OGRSpatialReferenceH * OGRSpatialReference::FindMatches(char **papszOptions, int *pnEntries, int **ppanMatchConfidence) const { + TAKE_OPTIONAL_LOCK(); + CPL_IGNORE_RET_VAL(papszOptions); if (pnEntries) @@ -11629,6 +11933,8 @@ OGRSpatialReference::FindMatches(char **papszOptions, int *pnEntries, OGRErr OGRSpatialReference::importFromEPSGA(int nCode) { + TAKE_OPTIONAL_LOCK(); + Clear(); const char *pszUseNonDeprecated = @@ -11738,6 +12044,8 @@ OGRErr OGRSpatialReference::importFromEPSGA(int nCode) */ OGRErr OGRSpatialReference::AddGuessedTOWGS84() { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); if (!d->m_pj_crs) return OGRERR_FAILURE; @@ -11869,6 +12177,8 @@ OGRErr CPL_STDCALL OSRImportFromEPSG(OGRSpatialReferenceH hSRS, int nCode) int OGRSpatialReference::EPSGTreatsAsLatLong() const { + TAKE_OPTIONAL_LOCK(); + if (!IsGeographic()) return FALSE; @@ -11978,6 +12288,8 @@ int OSREPSGTreatsAsLatLong(OGRSpatialReferenceH hSRS) int OGRSpatialReference::EPSGTreatsAsNorthingEasting() const { + TAKE_OPTIONAL_LOCK(); + if (!IsProjected()) return FALSE; @@ -12050,6 +12362,8 @@ OGRErr OGRSpatialReference::ImportFromESRIWisconsinWKT(const char *prjName, const char *unitsName, const char *crsName) { + TAKE_OPTIONAL_LOCK(); + if (centralMeridian < -93 || centralMeridian > -87) return OGRERR_FAILURE; if (latOfOrigin < 40 || latOfOrigin > 47) @@ -12201,6 +12515,8 @@ OGRErr OGRSpatialReference::ImportFromESRIWisconsinWKT(const char *prjName, */ OSRAxisMappingStrategy OGRSpatialReference::GetAxisMappingStrategy() const { + TAKE_OPTIONAL_LOCK(); + return d->m_axisMappingStrategy; } @@ -12239,6 +12555,8 @@ OSRAxisMappingStrategy OSRGetAxisMappingStrategy(OGRSpatialReferenceH hSRS) void OGRSpatialReference::SetAxisMappingStrategy( OSRAxisMappingStrategy strategy) { + TAKE_OPTIONAL_LOCK(); + d->m_axisMappingStrategy = strategy; d->refreshAxisMapping(); } @@ -12276,6 +12594,8 @@ void OSRSetAxisMappingStrategy(OGRSpatialReferenceH hSRS, */ const std::vector &OGRSpatialReference::GetDataAxisToSRSAxisMapping() const { + TAKE_OPTIONAL_LOCK(); + return d->m_axisMapping; } @@ -12343,6 +12663,8 @@ const int *OSRGetDataAxisToSRSAxisMapping(OGRSpatialReferenceH hSRS, OGRErr OGRSpatialReference::SetDataAxisToSRSAxisMapping( const std::vector &mapping) { + TAKE_OPTIONAL_LOCK(); + if (mapping.size() < 2) return OGRERR_FAILURE; d->m_axisMappingStrategy = OAMS_CUSTOM; @@ -12412,6 +12734,8 @@ bool OGRSpatialReference::GetAreaOfUse(double *pdfWestLongitudeDeg, double *pdfNorthLatitudeDeg, const char **ppszAreaName) const { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); if (!d->m_pj_crs) { @@ -12575,6 +12899,8 @@ void OSRDestroyCRSInfoList(OSRCRSInfo **list) */ void OGRSpatialReference::UpdateCoordinateSystemFromGeogCRS() { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); if (!d->m_pj_crs) return; @@ -12667,6 +12993,8 @@ void OGRSpatialReference::UpdateCoordinateSystemFromGeogCRS() */ OGRErr OGRSpatialReference::PromoteTo3D(const char *pszName) { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); if (!d->m_pj_crs) return OGRERR_FAILURE; @@ -12708,6 +13036,8 @@ OGRErr OSRPromoteTo3D(OGRSpatialReferenceH hSRS, const char *pszName) */ OGRErr OGRSpatialReference::DemoteTo2D(const char *pszName) { + TAKE_OPTIONAL_LOCK(); + d->refreshProjObj(); if (!d->m_pj_crs) return OGRERR_FAILURE; @@ -12749,6 +13079,8 @@ OGRErr OSRDemoteTo2D(OGRSpatialReferenceH hSRS, const char *pszName) int OGRSpatialReference::GetEPSGGeogCS() const { + TAKE_OPTIONAL_LOCK(); + /* -------------------------------------------------------------------- */ /* Check axis order. */ /* -------------------------------------------------------------------- */ From b4999bb5e828e83462931a620770225a346a590b Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Mon, 9 Sep 2024 09:49:48 +0200 Subject: [PATCH 8/9] GDALDataset::GetProjectionRef() and GetGCPProjection(): add locking on thread-safe dataset --- gcore/gdaldataset.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/gcore/gdaldataset.cpp b/gcore/gdaldataset.cpp index ee91b2d19302..2f17d2d69757 100644 --- a/gcore/gdaldataset.cpp +++ b/gcore/gdaldataset.cpp @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -114,6 +115,8 @@ class GDALDataset::Private GIntBig nTotalFeatures = TOTAL_FEATURES_NOT_INIT; OGRLayer *poCurrentLayer = nullptr; + std::mutex m_oMutexWKT{}; + char *m_pszWKTCached = nullptr; OGRSpatialReference *m_poSRSCached = nullptr; char *m_pszWKTGCPCached = nullptr; @@ -1166,6 +1169,11 @@ const char *GDALDataset::GetProjectionRef() const { return ""; } + + // If called on a thread-safe dataset, we might be called by several + // threads, so make sure our accesses to m_pszWKTCached are protected + // by a mutex. + std::lock_guard oLock(m_poPrivate->m_oMutexWKT); if (m_poPrivate->m_pszWKTCached && strcmp(pszWKT, m_poPrivate->m_pszWKTCached) == 0) { @@ -1831,6 +1839,11 @@ const char *GDALDataset::GetGCPProjection() { return ""; } + + // If called on a thread-safe dataset, we might be called by several + // threads, so make sure our accesses to m_pszWKTCached are protected + // by a mutex. + std::lock_guard oLock(m_poPrivate->m_oMutexWKT); if (m_poPrivate->m_pszWKTGCPCached && strcmp(pszWKT, m_poPrivate->m_pszWKTGCPCached) == 0) { From 580c6100872678e99a9c1a07954d9bc243c445d3 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Mon, 9 Sep 2024 11:00:25 +0200 Subject: [PATCH 9/9] GDALThreadSafeDataset: handle the case of methods that return non primitive types --- autotest/gcore/thread_test.py | 189 +++++++++++++++++++++++++++++++- gcore/gdalthreadsafedataset.cpp | 115 ++++++++++++++++++- 2 files changed, 302 insertions(+), 2 deletions(-) diff --git a/autotest/gcore/thread_test.py b/autotest/gcore/thread_test.py index f57281c00fc2..91e8d574b202 100755 --- a/autotest/gcore/thread_test.py +++ b/autotest/gcore/thread_test.py @@ -88,7 +88,7 @@ def verify_checksum(): res[0] = False assert False, (got_cs, expected_cs) - threads = [threading.Thread(target=verify_checksum)] + threads = [threading.Thread(target=verify_checksum) for i in range(2)] for t in threads: t.start() for t in threads: @@ -426,3 +426,190 @@ def test_thread_safe_unsupported_rat(): match="not supporting a non-GDALDefaultRasterAttributeTable implementation", ): ds.GetRasterBand(1).GetDefaultRAT() + + +def test_thread_safe_many_datasets(): + + tab_ds = [ + gdal.OpenEx( + "data/byte.tif" if (i % 3) < 2 else "data/utmsmall.tif", + gdal.OF_RASTER | gdal.OF_THREAD_SAFE, + ) + for i in range(100) + ] + + res = [True] + + def check(): + for _ in range(10): + for i, ds in enumerate(tab_ds): + if ds.GetRasterBand(1).Checksum() != (4672 if (i % 3) < 2 else 50054): + res[0] = False + + threads = [threading.Thread(target=check) for i in range(2)] + for t in threads: + t.start() + for t in threads: + t.join() + assert res[0] + + +def test_thread_safe_BeginAsyncReader(): + + with gdal.OpenEx("data/byte.tif", gdal.OF_RASTER | gdal.OF_THREAD_SAFE) as ds: + with pytest.raises(Exception, match="not supported"): + ds.BeginAsyncReader(0, 0, ds.RasterXSize, ds.RasterYSize) + + +def test_thread_safe_GetVirtualMem(): + + pytest.importorskip("numpy") + pytest.importorskip("osgeo.gdal_array") + + with gdal.OpenEx("data/byte.tif", gdal.OF_RASTER | gdal.OF_THREAD_SAFE) as ds: + with pytest.raises(Exception, match="not supported"): + ds.GetRasterBand(1).GetVirtualMemAutoArray(gdal.GF_Read) + + +def test_thread_safe_GetMetadadata(tmp_vsimem): + + filename = str(tmp_vsimem / "test.tif") + with gdal.GetDriverByName("GTiff").Create(filename, 1, 1) as ds: + ds.SetMetadataItem("foo", "bar") + ds.GetRasterBand(1).SetMetadataItem("bar", "baz") + + with gdal.OpenEx(filename, gdal.OF_RASTER | gdal.OF_THREAD_SAFE) as ds: + assert ds.GetMetadataItem("foo") == "bar" + assert ds.GetMetadataItem("not existing") is None + assert ds.GetMetadata() == {"foo": "bar"} + assert ds.GetMetadata("not existing") == {} + assert ds.GetRasterBand(1).GetMetadataItem("bar") == "baz" + assert ds.GetRasterBand(1).GetMetadataItem("not existing") is None + assert ds.GetRasterBand(1).GetMetadata() == {"bar": "baz"} + assert ds.GetRasterBand(1).GetMetadata("not existing") == {} + + +def test_thread_safe_GetUnitType(tmp_vsimem): + + filename = str(tmp_vsimem / "test.tif") + with gdal.GetDriverByName("GTiff").Create(filename, 1, 1) as ds: + ds.GetRasterBand(1).SetUnitType("foo") + + with gdal.OpenEx(filename, gdal.OF_RASTER | gdal.OF_THREAD_SAFE) as ds: + assert ds.GetRasterBand(1).GetUnitType() == "foo" + + +def test_thread_safe_GetColorTable(tmp_vsimem): + + filename = str(tmp_vsimem / "test.tif") + with gdal.GetDriverByName("GTiff").Create(filename, 1, 1) as ds: + ct = gdal.ColorTable() + ct.SetColorEntry(0, (1, 2, 3, 255)) + ds.GetRasterBand(1).SetColorTable(ct) + + with gdal.OpenEx(filename, gdal.OF_RASTER | gdal.OF_THREAD_SAFE) as ds: + res = [None] + + def thread_job(): + res[0] = ds.GetRasterBand(1).GetColorTable() + + t = threading.Thread(target=thread_job) + t.start() + t.join() + assert res[0] + assert res[0].GetColorEntry(0) == (1, 2, 3, 255) + ct = ds.GetRasterBand(1).GetColorTable() + assert ct.GetColorEntry(0) == (1, 2, 3, 255) + + +def test_thread_safe_GetSpatialRef(): + + with gdal.OpenEx("data/byte.tif", gdal.OF_RASTER | gdal.OF_THREAD_SAFE) as ds: + + res = [True] + + def check(): + for i in range(100): + + if len(ds.GetGCPs()) != 0: + res[0] = False + assert False + + if ds.GetGCPSpatialRef(): + res[0] = False + assert False + + if ds.GetGCPProjection(): + res[0] = False + assert False + + srs = ds.GetSpatialRef() + if not srs: + res[0] = False + assert False + if not srs.IsProjected(): + res[0] = False + assert False + if "NAD27 / UTM zone 11N" not in srs.ExportToWkt(): + res[0] = False + assert False + + if "NAD27 / UTM zone 11N" not in ds.GetProjectionRef(): + res[0] = False + assert False + + threads = [threading.Thread(target=check) for i in range(2)] + for t in threads: + t.start() + for t in threads: + t.join() + assert res[0] + + +def test_thread_safe_GetGCPs(): + + with gdal.OpenEx( + "data/byte_gcp_pixelispoint.tif", gdal.OF_RASTER | gdal.OF_THREAD_SAFE + ) as ds: + + res = [True] + + def check(): + for i in range(100): + + if len(ds.GetGCPs()) != 4: + res[0] = False + assert False + + gcp_srs = ds.GetGCPSpatialRef() + if gcp_srs is None: + res[0] = False + assert False + if not gcp_srs.IsGeographic(): + res[0] = False + assert False + if "unretrievable - using WGS84" not in gcp_srs.ExportToWkt(): + res[0] = False + assert False + + gcp_wkt = ds.GetGCPProjection() + if not gcp_wkt: + res[0] = False + assert False + if "unretrievable - using WGS84" not in gcp_wkt: + res[0] = False + assert False + + if ds.GetSpatialRef(): + res[0] = False + assert False + if ds.GetProjectionRef() != "": + res[0] = False + assert False + + threads = [threading.Thread(target=check) for i in range(2)] + for t in threads: + t.start() + for t in threads: + t.join() + assert res[0] diff --git a/gcore/gdalthreadsafedataset.cpp b/gcore/gdalthreadsafedataset.cpp index 3b2c8156f249..2ce0de26d31c 100644 --- a/gcore/gdalthreadsafedataset.cpp +++ b/gcore/gdalthreadsafedataset.cpp @@ -177,6 +177,71 @@ class GDALThreadSafeDataset final : public GDALProxyDataset static GDALDataset *Create(GDALDataset *poPrototypeDS, int nScopeFlags); + /* All below public methods override GDALDataset methods, and instead of + * forwarding to a thread-local dataset, they act on the prototype dataset, + * because they return a non-trivial type, that could be invalidated + * otherwise if the thread-local dataset is evicted from the LRU cache. + */ + const OGRSpatialReference *GetSpatialRef() const override + { + std::lock_guard oGuard(m_oPrototypeDSMutex); + if (m_oSRS.IsEmpty()) + { + auto poSRS = m_poPrototypeDS->GetSpatialRef(); + if (poSRS) + { + m_oSRS.AssignAndSetThreadSafe(*poSRS); + } + } + return m_oSRS.IsEmpty() ? nullptr : &m_oSRS; + } + + const OGRSpatialReference *GetGCPSpatialRef() const override + { + std::lock_guard oGuard(m_oPrototypeDSMutex); + if (m_oGCPSRS.IsEmpty()) + { + auto poSRS = m_poPrototypeDS->GetGCPSpatialRef(); + if (poSRS) + { + m_oGCPSRS.AssignAndSetThreadSafe(*poSRS); + } + } + return m_oGCPSRS.IsEmpty() ? nullptr : &m_oGCPSRS; + } + + const GDAL_GCP *GetGCPs() override + { + std::lock_guard oGuard(m_oPrototypeDSMutex); + return const_cast(m_poPrototypeDS)->GetGCPs(); + } + + const char *GetMetadataItem(const char *pszName, + const char *pszDomain = "") override + { + std::lock_guard oGuard(m_oPrototypeDSMutex); + return const_cast(m_poPrototypeDS) + ->GetMetadataItem(pszName, pszDomain); + } + + char **GetMetadata(const char *pszDomain = "") override + { + std::lock_guard oGuard(m_oPrototypeDSMutex); + return const_cast(m_poPrototypeDS) + ->GetMetadata(pszDomain); + } + + /* End of methods that forward on the prototype dataset */ + + GDALAsyncReader *BeginAsyncReader(int, int, int, int, void *, int, int, + GDALDataType, int, int *, int, int, int, + char **) override + { + CPLError(CE_Failure, CPLE_AppDefined, + "GDALThreadSafeDataset::BeginAsyncReader() not supported"); + return nullptr; + } + protected: GDALDataset *RefUnderlyingDataset() const override; @@ -190,7 +255,7 @@ class GDALThreadSafeDataset final : public GDALProxyDataset friend class GDALThreadLocalDatasetCache; /** Mutex that protects accesses to m_poPrototypeDS */ - std::mutex m_oPrototypeDSMutex{}; + mutable std::mutex m_oPrototypeDSMutex{}; /** "Prototype" dataset, that is the dataset that was passed to the * GDALThreadSafeDataset constructor. All calls on to it should be on @@ -208,6 +273,12 @@ class GDALThreadSafeDataset final : public GDALProxyDataset */ const CPLStringList m_aosThreadLocalConfigOptions{}; + /** Cached value returned by GetSpatialRef() */ + mutable OGRSpatialReference m_oSRS{}; + + /** Cached value returned by GetGCPSpatialRef() */ + mutable OGRSpatialReference m_oGCPSRS{}; + /** Structure that references all GDALThreadLocalDatasetCache* instances. */ struct GlobalCache @@ -275,6 +346,48 @@ class GDALThreadSafeRasterBand final : public GDALProxyRasterBand GDALRasterAttributeTable *GetDefaultRAT() override; + /* All below public methods override GDALRasterBand methods, and instead of + * forwarding to a thread-local dataset, they act on the prototype band, + * because they return a non-trivial type, that could be invalidated + * otherwise if the thread-local dataset is evicted from the LRU cache. + */ + const char *GetMetadataItem(const char *pszName, + const char *pszDomain = "") override + { + std::lock_guard oGuard(m_poTSDS->m_oPrototypeDSMutex); + return const_cast(m_poPrototypeBand) + ->GetMetadataItem(pszName, pszDomain); + } + + char **GetMetadata(const char *pszDomain = "") override + { + std::lock_guard oGuard(m_poTSDS->m_oPrototypeDSMutex); + return const_cast(m_poPrototypeBand) + ->GetMetadata(pszDomain); + } + + const char *GetUnitType() override + { + std::lock_guard oGuard(m_poTSDS->m_oPrototypeDSMutex); + return const_cast(m_poPrototypeBand)->GetUnitType(); + } + + GDALColorTable *GetColorTable() override + { + std::lock_guard oGuard(m_poTSDS->m_oPrototypeDSMutex); + return const_cast(m_poPrototypeBand)->GetColorTable(); + } + + /* End of methods that forward on the prototype band */ + + CPLVirtualMem *GetVirtualMemAuto(GDALRWFlag, int *, GIntBig *, + char **) override + { + CPLError(CE_Failure, CPLE_AppDefined, + "GDALThreadSafeRasterBand::GetVirtualMemAuto() not supported"); + return nullptr; + } + protected: GDALRasterBand *RefUnderlyingRasterBand(bool bForceOpen) const override; void UnrefUnderlyingRasterBand(