RFC-24 Mapscript memory management: call for comments

[Tamas Szekeres]

Umberto,

My personal attitude to this RFC is slightly ambivalent. On one hand I
appreciate your efforts to discover the similarities and the
differences between the various languages
regarding to a particular problem and it's also appealing that one of
our ancient problem had been tempting the mapscript users for a long
ago was included in the proposal.
But on the other hand one of the problems enumerated is not considered
to be a problem (by me), however a large amount of code should be
altered so as to "fight against it", and even I doubt that this
solution is working correctly as it stands.

Referring to the problems described in the RFC:

2.1 Object equality/identity

I consider this issue as a behaviour not a problem and I don't even
know about any bug related to it. I think when the user would use the
object after inserting to the parent, she can obtain the reference to
it with some additional code like:

mapObj map = new mapObj("mymap");
layerObj layer = new layerObj(null);
int index = map.insertLayer(layer, -1);
layer = map.getLayer(index);

or alternatively one should use the constructor with the parent object:

mapObj map = new mapObj("mymap");
layerObj layer = new layerObj(map);

2.2 Early garbage collection

That's really an issue, and a reference to the real memory owner
object should be kept by the child to avoid the crash.

2.3 (Constructors with parent object references)

That's also a serious problem and should be handled as described in
Bug 1743 (by altering the constructors of layerObj, classObj,
styleObj)


According to the comments above I would favour dealing only with 2.2 and 2.3 by

a, altering the 3 constructors (layerObj, classObj, styleObj)
b, keeping reference to the real memory owner objects at every object.

But I would highly discourage to retain internal references to the
child objects at the parents, because of the following reasons:

1, It requires large amount of code changes should be implemented
simultaneously by the various bindings that is a PITA.
2, No particular problems related to these changes since the child
objects are "disowned" by default and might be garbage collected
safely.
3, The RFC does not mention how msInsetLayer will be altered when the
following code is applied:

%typemap(javaout) int insertClass {
        // call the C API, which needs to be modified
        // so that the classObj is not copied anymore
        int actualIndex=$jnicall;
        classes[actualIndex]=classobj;
        // disown the classObj just inserted, item 3.3
        classobj.swigCMemOwn=false;
        return actualIndex;
}

To avoid using msCopyLayer I suppose a C struct copy to be done, so
'classobj' will hold different C struct as mapObj. By disowning
'classobj' a memory leak will possibly arise.

Getting back to the solution of b, (mentioned above) IMO this issue
might be considered as a bug in the SWIG implementation rather than in
the mapscript interface, so altering the SWIG default behaviour would
be expected instead of an application specific modification.
I had to deal with this issue at the GDAL C# bindings before, (because
some of the samples were definitely fail with this problem) and
altering how SWIG handles the ownership of the memory was a reasonable
option to solve this problem.
To safely handle this issue I've maintained the reference of the real
memory owner instead of the boolean swigCMemOwn parameter inside every
object. This reference (swigCMemOwner) is set to null if the memory
owned by the same object and should be disposed accordingly.
When swigCMemOwner is not null the underlying C memory is not to be
freed at all.
The related changes can be found in the
gdal/swig/include/csharp/swig_csharp_extensions.i file of the GDAL
project tree.



Best Regards,



2007/1/10, Umberto Nicoletti <umberto.nicoletti at gmail.com>:
> Developers,
> I have spent by now quite some time on the writing of RFC-24 and I
> need to hear your opinions before I take it further with more
> examples, more details, code or anything else is needed.
> I am therefore opening RFC 24 for comments.
> When I have enough feedback I'll make the needed changes and then
> submit it for voting.
>
> RFC-24 is available at the usual location:
> http://mapserver.gis.umn.edu/development/rfc/ms-rfc-24
>
> Regards,
> Umberto
>