[gdal-dev] Locked files under Windows

[Roger Bivand]

On Mon, 2 Dec 2013, Even Rouault wrote:

> 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.

Yes, this works. Both pDriver and filename must be separated from their 
origins in pDataset before pDataset is closed.

Thanks again,

Roger

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

-- 
Roger Bivand
Department of Economics, Norwegian School of Economics,
Helleveien 30, N-5045 Bergen, Norway.
voice: +47 55 95 93 55; fax +47 55 95 95 43
e-mail: Roger.Bivand at nhh.no