[gdal-dev] Locked files under Windows

[Even Rouault]

Le lundi 02 décembre 2013 11:22:11, Roger Bivand a écrit :
> On Tue, 5 Nov 2013, Roger Bivand wrote:
> > On Tue, 5 Nov 2013, Even Rouault wrote:
> >>> Even,
> >>> 
> >>> Thanks as always for getting back quickly! Yes, the relevant code is
> >>> (line
> >>> 
> >>> 475 in rgdal/src/gdal-bindings.cpp):
> >>>    GDALDataset *pDataset =
> >>>    
> >>>      (GDALDataset *) R_ExternalPtrAddr(sxpHandle);
> >>>    
> >>>    if (pDataset == NULL) return(R_NilValue);
> >>>    
> >>>    installErrorHandler();
> >>>    
> >>>    GDALDriver *pDriver = pDataset->GetDriver();
> >>>    
> >>>    const char *filename = pDataset->GetDescription();
> >>>    
> >>>    GDALDeleteDataset((GDALDriverH) pDriver, filename);
> >>>    
> >>>    R_ClearExternalPtr(sxpHandle);
> >>>    
> >>>    uninstallErrorHandlerAndTriggerError();
> >>> 
> >>> but after this is run, the file remains open. Is GDALDeleteDataset()
> >>> inappropriate? Should we say GDALClose((GDALDatasetH)pDataset); first;
> >>> you suggested this instead of pDataset->~GDALDataset(); last year? The
> >>> file pointers are inside the Dataset objects, so obviously they should
> >>> be closed. We have tried putting things in various orders, without
> >>> success so far, I'm afraid.
> >> 
> >> GDALDeleteDataset() doesn't take the dataset object as a parameter, so
> >> it will
> >> not delete it for you (in the C++ sense of delete... The naming of
> >> functions
> >> is a bit confusing admitedly)
> >> 
> >> Yes, you need to call GDALClose() before GDALDeleteDataset(), and make
> >> sure that the handle to the dataset is no used afterwards of course. I
> >> mention that
> >> because I'm not sure what R_ClearExternalPtr() does.
> > 
> > Right - I've been trying this based on your first reply, which put me "on
> > track". I think that we have assumed that GDALDeleteDataset() took an
> > open dataset and deleted it, closing, freeing and unlinking combined. It
> > makes sense, though, to close first after having stored the arguments to
> > GDALDeleteDataset(). The R_ClearExternalPtr() function clears the
> > internal object in R in which external pointers are stored, so would
> > zombie the dataset if applied too early. Initial tests on Linux and
> > Windows are promising ...
> 
> Even,
> 
> Not end of story, I'm afraid. Running GCC 4.8.2 AddressSanitizer, we see:
> > GDAL.close(x)
> 
> =================================================================
> ==30754== ERROR: AddressSanitizer: heap-use-after-free on address
> ...
>      #5 0x7ffd14e195db in RGDAL_DeleteHandle
> /data/gannet/ripley/R/packages/tests-48x/rgdal/src/gdal-bindings.cpp:491
>      #6 0x7ffd14e19675 in RGDAL_CloseDataset
> /data/gannet/ripley/R/packages/tests-48x/rgdal/src/gdal-bindings.cpp:519
> 
> (full report at foot of:
> http://www.stats.ox.ac.uk/pub/bdr/memtests/ASAN/rgdal.out)
> 
> where RGDAL_DeleteHandle after revision is:
> 
>    installErrorHandler();
> 
>    GDALDriver *pDriver = pDataset->GetDriver();
>    const char *desc = GDALGetDriverShortName(pDriver);
>    GDALDriver *pDriver1 = (GDALDriver *) GDALGetDriverByName(desc);
> 
>    const char *filename = pDataset->GetDescription();
> 
>    GDALClose((GDALDatasetH) pDataset);
>    GDALDeleteDataset((GDALDriverH) pDriver1, filename);
> 
>    R_ClearExternalPtr(sxpHandle);
>    uninstallErrorHandlerAndTriggerError();
> 
> 
> (R_ClearExternalPtr removes the external pointer stored as an R workspace
> object).
> 
> I suspect that ASAN is unhappy about pDriver being a pointer inside
> pDataset when pDataset no longer exists. It is being over-touchy, maybe,
> but if the heap was overwritten between pDriver being retrieved and used,
> the consequences might be unpredictable. I've tried making pDriver
> independent of pDataset, as you see, but the ASAN error is the same. Is
> filename a copy or does it point into pDataset?

You can remove the following lines and just use pDriver
>    const char *desc = GDALGetDriverShortName(pDriver);
>    GDALDriver *pDriver1 = (GDALDriver *) GDALGetDriverByName(desc);

The error is that the pointer returned by GetDescription() is owned by the 
dataset, so becomes invalid when you close it. You have to strdup() the value 
before closing the dataset.

> 
> Best wishes,
> 
> Roger
> 
> > Thanks again,
> > 
> > Roger
> > 
> >>> Best wishes,
> >>> 
> >>> Roger

-- 
Geospatial professional services
http://even.rouault.free.fr/services.html