[gdal-dev] Regarding to the memory management in the SWIG API

[Ari Jolma]

On 01/08/2014 12:27 PM, Even Rouault wrote:
> Le mercredi 08 janvier 2014 11:10:58, Tamas Szekeres a écrit :
>> Hi Devs,
>>
>> Is that intentional that we don't call ReleaseResultset within the
>> destructor of the OGR Layer object?
> The OGR Layer object has generally no pointer to the datasource from which it
> originates (well, at the swig level we could probably attach one), so you
> cannot call ReleaseResultSet


In the Perl bindings a link between OGR datasource and layer objects is 
maintained and there is an explicit destructor for layer objects, which 
may implicitly destruct a datasource object if one is left dangling. 
Implicit because it is left for Perl to decide.

It's a good point that the ReleaseResultSet should be made a no-op and 
hide it into the layer destructor. I'll look into implementing it in the 
Perl bindings.

I assume it would be a good thing to implement this kind of machinery in 
the Swig bindings but it may be a complex task.

Ari


>
>> Actually the destructor is not even
>> generated (at least for the C# API)
> That must be the case of all bindings, because the OGR Layer object has no
> destructor in its API. OGR C++ Layers obtained by OGRDataSource::GetLayer()
> are owned by the C++ OGRDataSource and destroyed by it.
>
>> which seems to be an improper solution
>> and leads to memory leaks, definitely. However ReleaseResultset is exposed
>> in the API, but it's fairly dangerous to let the user decide whether to
>> call this method or not.
> Yes, that's not very popular in garbage collected languages to let the user
> deal with memory management...
>
>> In my understanding the destructor of the Layer object should look
>> something like the destructor of the DataSource (but calling
>> ReleaseResultset instead of ReleaseDatasource).
> Actually you should distinguish how the Layer object has been created : if it
> comes from GetLayer() or ExecuteSQL()
>
>> However modifying the code
>> this way, may lead to a crash for the existing code which call
>> ReleaseResultset directly.
> Yes, that would cause breakage in all existing code. The alternative would be
> that the public SWIG ReleaseResultSet() becomes a no-op and the destructor of
> SWIG Layer object call OGR_DS_ReleaseResultSet() when appropriate.
>
>> Any opinion?
>>
>> Best regards,
>>
>> Tamas