diff --git a/autotest/gdrivers/netcdf_multidim.py b/autotest/gdrivers/netcdf_multidim.py index c7c03b2ce90e..c4ebf06551fc 100755 --- a/autotest/gdrivers/netcdf_multidim.py +++ b/autotest/gdrivers/netcdf_multidim.py @@ -2613,3 +2613,19 @@ def create(): assert delay < 1 gdal.Unlink("tmp/test_netcdf_multidim_read_transposed_bigger_file.nc") + + +def test_netcdf_multidim_var_alldatatypes_opened_twice(): + """Test https://github.com/OSGeo/gdal/pull/6311""" + + ds1 = gdal.OpenEx("data/netcdf/alldatatypes.nc", gdal.OF_MULTIDIM_RASTER) + assert ds1 + rg1 = ds1.GetRootGroup() + assert rg1 + assert rg1.OpenMDArray("string_var") is not None + + ds2 = gdal.OpenEx("data/netcdf/alldatatypes.nc", gdal.OF_MULTIDIM_RASTER) + assert ds2 + rg2 = ds2.GetRootGroup() + assert rg2 + assert rg2.OpenMDArray("string_var") is not None diff --git a/frmts/netcdf/netcdfdataset.cpp b/frmts/netcdf/netcdfdataset.cpp index a3aa1ae120fa..a5760e190c65 100644 --- a/frmts/netcdf/netcdfdataset.cpp +++ b/frmts/netcdf/netcdfdataset.cpp @@ -162,6 +162,57 @@ static CPLErr NCDFGetCoordAndBoundVarFullNames( int nCdfId, CPLMutex *hNCMutex = nullptr; +// Workaround https://github.com/OSGeo/gdal/issues/6253 +// Having 2 netCDF handles on the same file doesn't work in a multi-threaded +// way. Apparently having the same handle works better (this is OK since +// we have a global mutex on the netCDF library) +static std::map goMapNameToNetCDFId; +static std::map> goMapNetCDFIdToKeyAndCount; + +int GDAL_nc_open(const char* pszFilename, int nMode, int* pID) +{ + std::string osKey(pszFilename); + osKey += "#####"; + osKey += std::to_string(nMode); + auto oIter = goMapNameToNetCDFId.find(osKey); + if( oIter == goMapNameToNetCDFId.end() ) + { + int ret = nc_open(pszFilename, nMode, pID); + if( ret != NC_NOERR ) + return ret; + goMapNameToNetCDFId[osKey] = *pID; + goMapNetCDFIdToKeyAndCount[*pID] = std::pair(osKey, 1); + return ret; + } + else + { + *pID = oIter->second; + goMapNetCDFIdToKeyAndCount[oIter->second].second ++; + return NC_NOERR; + } +} + +int GDAL_nc_close(int cdfid) +{ + int ret = NC_NOERR; + auto oIter = goMapNetCDFIdToKeyAndCount.find(cdfid); + if( oIter != goMapNetCDFIdToKeyAndCount.end() ) + { + if( --oIter->second.second == 0 ) + { + ret = nc_close(cdfid); + goMapNameToNetCDFId.erase(oIter->second.first); + goMapNetCDFIdToKeyAndCount.erase(oIter); + } + } + else + { + // we can go here if file opened with nc_open_mem() or nc_create() + ret = nc_close(cdfid); + } + return ret; +} + /************************************************************************/ /* ==================================================================== */ /* netCDFRasterBand */ @@ -2834,7 +2885,7 @@ netCDFDataset::~netCDFDataset() #ifdef NCDF_DEBUG CPLDebug("GDAL_netCDF", "calling nc_close( %d)", cdfid); #endif - int status = nc_close(cdfid); + int status = GDAL_nc_close(cdfid); #ifdef ENABLE_UFFD NETCDF_UFFD_UNMAP(pCtx); #endif @@ -7472,7 +7523,7 @@ bool netCDFDataset::GrowDim(int nLayerId, int nDimIdToGrow, size_t nNewSize) eFormat == NCDF_FORMAT_NC4, nLayerId, nDimIdToGrow, nNewSize) ) { - nc_close(new_cdfid); + GDAL_nc_close(new_cdfid); return false; } @@ -7498,7 +7549,7 @@ bool netCDFDataset::GrowDim(int nLayerId, int nDimIdToGrow, size_t nNewSize) if( status != NC_NOERR ) { CPLFree(panGroupIds); - nc_close(new_cdfid); + GDAL_nc_close(new_cdfid); return false; } if( !CloneGrp(panGroupIds[i], nNewGrpId, @@ -7506,7 +7557,7 @@ bool netCDFDataset::GrowDim(int nLayerId, int nDimIdToGrow, size_t nNewSize) nLayerId, nDimIdToGrow, nNewSize) ) { CPLFree(panGroupIds); - nc_close(new_cdfid); + GDAL_nc_close(new_cdfid); return false; } } @@ -7527,9 +7578,9 @@ bool netCDFDataset::GrowDim(int nLayerId, int nDimIdToGrow, size_t nNewSize) } #endif - nc_close(cdfid); + GDAL_nc_close(cdfid); cdfid = -1; - nc_close(new_cdfid); + GDAL_nc_close(new_cdfid); CPLString osOriFilename(osFilename + ".ori"); if( VSIRename(osFilename, osOriFilename) != 0 || @@ -7549,7 +7600,7 @@ bool netCDFDataset::GrowDim(int nLayerId, int nDimIdToGrow, size_t nNewSize) CPLFree(pszTemp); } #endif - status = nc_open(osFilenameForNCOpen, NC_WRITE, &cdfid); + status = GDAL_nc_open(osFilenameForNCOpen, NC_WRITE, &cdfid); NCDF_ERR(status); if( status != NC_NOERR ) return false; @@ -8281,7 +8332,7 @@ bool netCDFDatasetCreateTempFile( NetCDFFormatEnum eFormat, } } - nc_close(nCdfId); + GDAL_nc_close(nCdfId); return true; } @@ -8517,9 +8568,9 @@ GDALDataset *netCDFDataset::Open( GDALOpenInfo *poOpenInfo ) status2 = nc_open_mem(CPLGetFilename(osFilenameForNCOpen), nMode, static_cast(nVmaSize), pVma, &cdfid); } else - status2 = nc_open(osFilenameForNCOpen, nMode, &cdfid); + status2 = GDAL_nc_open(osFilenameForNCOpen, nMode, &cdfid); #else - status2 = nc_open(osFilenameForNCOpen, nMode, &cdfid); + status2 = GDAL_nc_open(osFilenameForNCOpen, nMode, &cdfid); #endif } if( status2 != NC_NOERR ) @@ -8604,7 +8655,7 @@ GDALDataset *netCDFDataset::Open( GDALOpenInfo *poOpenInfo ) "%s is a netCDF file, but %s is not a variable.", poOpenInfo->pszFilename, osSubdatasetName.c_str()); - nc_close(cdfid); + GDAL_nc_close(cdfid); #ifdef ENABLE_UFFD NETCDF_UFFD_UNMAP(pCtx); #endif diff --git a/frmts/netcdf/netcdfdataset.h b/frmts/netcdf/netcdfdataset.h index bb233e119fe5..078df68cae5f 100644 --- a/frmts/netcdf/netcdfdataset.h +++ b/frmts/netcdf/netcdfdataset.h @@ -1126,4 +1126,7 @@ bool netCDFDatasetCreateTempFile( NetCDFFormatEnum eFormat, VSILFILE* fpSrc ); #endif +int GDAL_nc_open(const char* pszFilename, int nMode, int* pID); +int GDAL_nc_close(int cdfid); + #endif diff --git a/frmts/netcdf/netcdfmultidim.cpp b/frmts/netcdf/netcdfmultidim.cpp index cd6175a84d4b..fddffe775814 100644 --- a/frmts/netcdf/netcdfmultidim.cpp +++ b/frmts/netcdf/netcdfmultidim.cpp @@ -508,7 +508,7 @@ netCDFSharedResources::~netCDFSharedResources() #ifdef NCDF_DEBUG CPLDebug("GDAL_netCDF", "calling nc_close( %d)", m_cdfid); #endif - int status = nc_close(m_cdfid); + int status = GDAL_nc_close(m_cdfid); NCDF_ERR(status); } @@ -3245,6 +3245,7 @@ std::vector netCDFVariable::GetBlockSize() const // We add 1 to the dimension count, for 2D char variables that we // expose as a 1D variable. std::vector anTemp(1 + nDimCount); + CPLMutexHolderD(&hNCMutex); nc_inq_var_chunking(m_gid, m_varid, &nStorageType, &anTemp[0]); if( nStorageType == NC_CHUNKED ) { @@ -4007,10 +4008,10 @@ GDALDataset *netCDFDataset::OpenMultiDim( GDALOpenInfo *poOpenInfo ) status2 = nc_open_mem(CPLGetFilename(osFilenameForNCOpen), nMode, static_cast(nVmaSize), pVma, &cdfid); } else - status2 = nc_open(osFilenameForNCOpen, nMode, &cdfid); + status2 = GDAL_nc_open(osFilenameForNCOpen, nMode, &cdfid); poSharedResources->m_pUffdCtx = pCtx; #else - status2 = nc_open(osFilenameForNCOpen, nMode, &cdfid); + status2 = GDAL_nc_open(osFilenameForNCOpen, nMode, &cdfid); #endif } if( status2 != NC_NOERR )