RFC-24 Mapscript memory management: call for comments

[Umberto Nicoletti]

On 1/19/07, Tamas Szekeres <szekerest at gmail.com> wrote:
> 2007/1/19, Steve Lime <Steve.Lime at dnr.state.mn.us>:
> > Seems like most of the issues are related to not knowing who (e.g. Swig
> > or a mapObj)
> > owns an object. It seems that if we removed ambiguity there then that
> > would really
> > help and you wouldn't have to touch the Swig stuff much.
> >
> > That is:
> >
> >   - use dynamically allocated arrays of layers, classes and styles
> >
> >   - then change insert functions to take ownership of objects to be
> > inserted (I assume
> > this is much easier with a list of pointers) (the insert sets parent
> > pointer)
> >
> >   - with that change wouldn't the deconstructors work "as is"? That is,
> > ok to free
> > if parent is NULL?
> >
>
>
> At the C side, if the parent is treated as the owner of the child, the
> parent should not be destroyed until there are valid references to the
> childs at the target language. In this regard, there's much more
> things to do so as to implement this concept correctly. Currently the
> greatest deal is to notify the wrapper when owner of the object (being
> responsible to destroy the underlying memory)  changes.
>
>
>
> Steve,
>
> Reading this thread, I can see a general disapproval of treating the
> problems at the various target languages rather than trying to modify
> the C side of the code somehow. It would be quite an easy to handle
> the memory management uniquely at the C side without the need of
> discovering the various aspects of a particular interface.
>
> IMO, to implement this concept the lifespan of the C structures and
> the corresponding wrappers at the target language should be decoupled.
> Roughly speaking an additional property holding the number of
> references should be added to every structures being exposed to the
> SWIG interface, and the target language should notify the structure
> about the release of this reference. Here are my steps to implement
> this kind of functionality:
>
>
> 1. Use dynamically allocated objects (arrays of layers, classes and
> styles) instead of nested structures.
> (Using nested structures would predestinate the parent as owner of the
> memory of the object)
>
> 2. Add a refcount member to every structure to be exposed to the
> target language (the initial value is =1)
>
> 3. Alter the destroy logic of the structures. The structure should
> only be destroyed when the reference count reaches zero, like:
>
> void msFreeMap(mapObj *map) {
>   if(!map) return;
>   if (--map->refcount > 0) return;
>   // destroying the object as usual
> }
>
> 4. Change the wrappers to consider every object is self-owned ensuring
> that the C destroy function will be called when the wrapper object is
> garbage collected and destroyed. To acieve this, the followings should
> be done:
>
> a, %newobject should be added to every members returning object
> references.The functions should increment the refcount of the
> corresponding structure.
> b, When the structures have object reference members to be exposed to
> the SWIG interface (like the reference to the mapObj at the layerObj),
> the members should be replaced with the corresponding getters/setters.
> These accessors should  increment the refcount on the structure.
> (potential backward incompatible change)
> c, At the member functions taking over the objects passed as
> parameters (like insertLayer) should increment the refcount of the
> object parameter.
>
> 5. When a parent is destroyed the back references of the childs to
> this parent should be set to null.
>

Tamas,
I am afraid this might not work. Consider this for example:

1) in function 'a' a new mapObj is allocated
2) the mapObj is passed to function 'b' which allocates a new layer
and adds it to the map. The new layer is a local variable, suitable
for gc after method 'b' returns

at step 1 the mapObj has refcount=1, after step 2 the layerObj has
refcount=2 (1 for itself and 1 from the map).

When function 'b' returns the gc deletes the layer object. The
destructor will not deallocate memory because (--refcount>0) evaluates
to false.
The map object will also not release this memory because every object
is responsible for itself, but the object responsible for the layer
memory has already been GC'ed.

I would go as far as stating that reference counting will NEVER work.
In fact the only language that still uses a main gc based on reference
counting is Perl and it is explicitly said in the docs that it might
create memory leaks when there are cyclic references.
To be fair Perl also has another gc that can detect these leaks, but
it kicks in only when a thread ends or the vm exits.

Umberto

>
> Some other things might also be considered.
>
>
> Best Regards,
>
> Tamas
>