Skip to content

Commit

Permalink
Merge pull request #9253 from rouault/fix_1837
Browse files Browse the repository at this point in the history 
GDALGetCacheMax64(): fix TSAN complaint (fixes #1837)
  • Loading branch information
@rouault
rouault authored Feb 26, 2024
2 parents 6095912 + a1ba66f commit c30205a
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 84 deletions.
116 changes: 59 additions & 57 deletions gcore/gdalrasterblock.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include <algorithm>
#include <climits>
#include <cstring>
#include <mutex>

#include "cpl_atomic_ops.h"
#include "cpl_conv.h"
Expand All @@ -43,7 +44,6 @@
#include "cpl_string.h"
#include "cpl_vsi.h"

static bool bCacheMaxInitialized = false;
// Will later be overridden by the default 5% if GDAL_CACHEMAX not defined.
static GIntBig nCacheMax = 40 * 1024 * 1024;
static GIntBig nCacheUsed = 0;
Expand Down Expand Up @@ -153,10 +153,8 @@ void CPL_STDCALL GDALSetCacheMax64(GIntBig nNewSizeInBytes)
}
#endif

{
INITIALIZE_LOCK;
}
bCacheMaxInitialized = true;
// To force one-time initialization of nCacheMax if not already done
GDALGetCacheMax64();
nCacheMax = nNewSizeInBytes;

/* -------------------------------------------------------------------- */
Expand Down Expand Up @@ -237,73 +235,77 @@ int CPL_STDCALL GDALGetCacheMax()

GIntBig CPL_STDCALL GDALGetCacheMax64()
{
if (!bCacheMaxInitialized)
{
static std::once_flag flagSetupGDALGetCacheMax64;
std::call_once(
flagSetupGDALGetCacheMax64,
[]()
{
INITIALIZE_LOCK;
}
bSleepsForBockCacheDebug =
CPLTestBool(CPLGetConfigOption("GDAL_DEBUG_BLOCK_CACHE", "NO"));
{
INITIALIZE_LOCK;
}
bSleepsForBockCacheDebug =
CPLTestBool(CPLGetConfigOption("GDAL_DEBUG_BLOCK_CACHE", "NO"));

const char *pszCacheMax = CPLGetConfigOption("GDAL_CACHEMAX", "5%");
const char *pszCacheMax = CPLGetConfigOption("GDAL_CACHEMAX", "5%");

GIntBig nNewCacheMax;
if (strchr(pszCacheMax, '%') != nullptr)
{
GIntBig nUsablePhysicalRAM = CPLGetUsablePhysicalRAM();
if (nUsablePhysicalRAM > 0)
GIntBig nNewCacheMax;
if (strchr(pszCacheMax, '%') != nullptr)
{
// For some reason, coverity pretends that this will overflow.
// "Multiply operation overflows on operands
// static_cast<double>( nUsablePhysicalRAM ) and
// CPLAtof(pszCacheMax). Example values for operands: CPLAtof(
// pszCacheMax ) = 2251799813685248,
// static_cast<double>(nUsablePhysicalRAM) =
// -9223372036854775808." coverity[overflow,tainted_data]
double dfCacheMax = static_cast<double>(nUsablePhysicalRAM) *
CPLAtof(pszCacheMax) / 100.0;
if (dfCacheMax >= 0 && dfCacheMax < 1e15)
nNewCacheMax = static_cast<GIntBig>(dfCacheMax);
GIntBig nUsablePhysicalRAM = CPLGetUsablePhysicalRAM();
if (nUsablePhysicalRAM > 0)
{
// For some reason, coverity pretends that this will overflow.
// "Multiply operation overflows on operands
// static_cast<double>( nUsablePhysicalRAM ) and
// CPLAtof(pszCacheMax). Example values for operands: CPLAtof(
// pszCacheMax ) = 2251799813685248,
// static_cast<double>(nUsablePhysicalRAM) =
// -9223372036854775808." coverity[overflow,tainted_data]
double dfCacheMax =
static_cast<double>(nUsablePhysicalRAM) *
CPLAtof(pszCacheMax) / 100.0;
if (dfCacheMax >= 0 && dfCacheMax < 1e15)
nNewCacheMax = static_cast<GIntBig>(dfCacheMax);
else
nNewCacheMax = nCacheMax;
}
else
{
CPLDebug("GDAL", "Cannot determine usable physical RAM.");
nNewCacheMax = nCacheMax;
}
}
else
{
CPLDebug("GDAL", "Cannot determine usable physical RAM.");
nNewCacheMax = nCacheMax;
}
}
else
{
nNewCacheMax = CPLAtoGIntBig(pszCacheMax);
if (nNewCacheMax < 100000)
{
if (nNewCacheMax < 0)
nNewCacheMax = CPLAtoGIntBig(pszCacheMax);
if (nNewCacheMax < 100000)
{
CPLError(CE_Failure, CPLE_NotSupported,
"Invalid value for GDAL_CACHEMAX. "
"Using default value.");
GIntBig nUsablePhysicalRAM = CPLGetUsablePhysicalRAM();
if (nUsablePhysicalRAM)
nNewCacheMax = nUsablePhysicalRAM / 20;
if (nNewCacheMax < 0)
{
CPLError(CE_Failure, CPLE_NotSupported,
"Invalid value for GDAL_CACHEMAX. "
"Using default value.");
GIntBig nUsablePhysicalRAM = CPLGetUsablePhysicalRAM();
if (nUsablePhysicalRAM)
nNewCacheMax = nUsablePhysicalRAM / 20;
else
{
CPLDebug("GDAL",
"Cannot determine usable physical RAM.");
nNewCacheMax = nCacheMax;
}
}
else
{
CPLDebug("GDAL",
"Cannot determine usable physical RAM.");
nNewCacheMax = nCacheMax;
nNewCacheMax *= 1024 * 1024;
}
}
else
{
nNewCacheMax *= 1024 * 1024;
}
}
}
nCacheMax = nNewCacheMax;
CPLDebug("GDAL", "GDAL_CACHEMAX = " CPL_FRMT_GIB " MB",
nCacheMax / (1024 * 1024));
bCacheMaxInitialized = true;
}
nCacheMax = nNewCacheMax;
CPLDebug("GDAL", "GDAL_CACHEMAX = " CPL_FRMT_GIB " MB",
nCacheMax / (1024 * 1024));
});

// coverity[overflow_sink]
return nCacheMax;
}
Expand Down
52 changes: 25 additions & 27 deletions port/cpl_multiproc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,16 +358,16 @@ int CPLCreateOrAcquireMutexEx(CPLMutex **phMutex, double dfWaitInSeconds,
/************************************************************************/

#ifdef MUTEX_NONE
static int CPLCreateOrAcquireMutexInternal(CPLLock **phLock,
double dfWaitInSeconds,
CPLLockType eType)
static bool CPLCreateOrAcquireMutexInternal(CPLLock **phLock,
double dfWaitInSeconds,
CPLLockType eType)
{
return false;
}
#else
static int CPLCreateOrAcquireMutexInternal(CPLLock **phLock,
double dfWaitInSeconds,
CPLLockType eType)
static bool CPLCreateOrAcquireMutexInternal(CPLLock **phLock,
double dfWaitInSeconds,
CPLLockType eType)

{
bool bSuccess = false;
Expand Down Expand Up @@ -1462,35 +1462,31 @@ int CPLCreateOrAcquireMutexEx(CPLMutex **phMutex, double dfWaitInSeconds,
int nOptions)

{
bool bSuccess = false;

pthread_mutex_lock(&global_mutex);
if (*phMutex == nullptr)
{
*phMutex = CPLCreateMutexInternal(true, nOptions);
bSuccess = *phMutex != nullptr;
const bool bSuccess = *phMutex != nullptr;
pthread_mutex_unlock(&global_mutex);
if (!bSuccess)
return false;
}
else
{
pthread_mutex_unlock(&global_mutex);

bSuccess = CPL_TO_BOOL(CPLAcquireMutex(*phMutex, dfWaitInSeconds));
}

return bSuccess;
return CPL_TO_BOOL(CPLAcquireMutex(*phMutex, dfWaitInSeconds));
}

/************************************************************************/
/* CPLCreateOrAcquireMutexInternal() */
/************************************************************************/

static int CPLCreateOrAcquireMutexInternal(CPLLock **phLock,
double dfWaitInSeconds,
CPLLockType eType)
static bool CPLCreateOrAcquireMutexInternal(CPLLock **phLock,
double dfWaitInSeconds,
CPLLockType eType)
{
bool bSuccess = false;

pthread_mutex_lock(&global_mutex);
if (*phLock == nullptr)
{
Expand All @@ -1507,18 +1503,17 @@ static int CPLCreateOrAcquireMutexInternal(CPLLock **phLock,
*phLock = nullptr;
}
}
bSuccess = *phLock != nullptr;
const bool bSuccess = *phLock != nullptr;
pthread_mutex_unlock(&global_mutex);
if (!bSuccess)
return false;
}
else
{
pthread_mutex_unlock(&global_mutex);

bSuccess =
CPL_TO_BOOL(CPLAcquireMutex((*phLock)->u.hMutex, dfWaitInSeconds));
}

return bSuccess;
return CPL_TO_BOOL(CPLAcquireMutex((*phLock)->u.hMutex, dfWaitInSeconds));
}

/************************************************************************/
Expand Down Expand Up @@ -1624,20 +1619,23 @@ static CPLMutex *CPLCreateMutexInternal(bool bAlreadyInGlobalLock, int nOptions)
psItem->nOptions = nOptions;
CPLInitMutex(psItem);

// Mutexes are implicitly acquired when created.
CPLAcquireMutex(reinterpret_cast<CPLMutex *>(psItem), 0.0);

return reinterpret_cast<CPLMutex *>(psItem);
}

CPLMutex *CPLCreateMutex()
{
return CPLCreateMutexInternal(false, CPL_MUTEX_RECURSIVE);
CPLMutex *mutex = CPLCreateMutexInternal(false, CPL_MUTEX_RECURSIVE);
if (mutex)
CPLAcquireMutex(mutex, 0);
return mutex;
}

CPLMutex *CPLCreateMutexEx(int nOptions)
{
return CPLCreateMutexInternal(false, nOptions);
CPLMutex *mutex = CPLCreateMutexInternal(false, nOptions);
if (mutex)
CPLAcquireMutex(mutex, 0);
return mutex;
}

/************************************************************************/
Expand Down

0 comments on commit c30205a

Please sign in to comment.