[RFC-24] Updated public draft

[Tamas Szekeres]

2007/2/25, Umberto Nicoletti <umberto.nicoletti at gmail.com>:
>
> I have checked and it seems that we can add a check at the beginning
> of all functions where a layer uncorrectly dereferences the map
> pointer without checking. The check will be  like this:
>
> if (layer->map == NULL) {
>     // should we raise MS_CHILDERR instead?
>     msSetError(MS_MEMERR, "map is null", "");
>     return MS_FAILURE;
> }
>

Umberto,

I've spent some time tracking down the relationship between the
various mapscript classes and even an UML diagram have been created
for this purpose.
Currently the lifetime of many of the objects are controlled by other objects.
Some of the objects can be considered as 'root object' and can control
the destruction of its own memory safely. However many of the classes
are considered as descendant of the root and should eventually be
destroyed by the root object.
Furthermore some of the classes (like hashtableObj, colorObj,
labelObj, rectObj) may belong to various class types.
Some of the classes may appear as root and can also appear as
descendant class (like shapeObj, pointObj, projectionObj, rectObj)

In this regard the current memory management model cannot be altered
safely and would require large amount of work to implement. Every
nested class members should be rewritten as dynamically allocated
members in this case.
In addition the issue how the back-references are handled denotes
obviously that we should not bother with this kind of work anymore. In
addition it's not the best way to treat
the problem as raising an error when the back reference is null for a
particular
situation, because the user cannot see obviously why that error have
occurred and how to protect against it.
When a particular class is considered as a potential child class, it's
definitely unsafe to support the separation of this class, and change
the ownership of the memory at run time.

I would favour making some stripping on the interface to properly
reflect how the objects are managed internally.
Some of the objects should not be externally creatable (layerObj,
classObj for example) and some members should not be writable etc.

According to the issues mentioned above to properly implement a
reference counting approach at the C side some kind of
object memory context management should be introduced. All of the
dependent object should be assigned to the same context at run time.
Though the framework of this concept could be created easily, the
proper run time assignment of these contexts would require large
amount of coding as I can see and would destabilize the current
implementation.

>
> If we correct this do we still need to keep a parent reference in the
> child mapscript object?
>

Currenly I would rather implement this parent reference approach at the target
language side in a generalized fashion. Recall that a particular type
may belong to various other types so this parent reference cannot be a
strongly typed reference to a specific mapscript class type.
I've attached my solution specifically for the C# interface but it
might be easily rewritten to java as well.
This approach appears to operate for the GDAL C# fairly well.
Here's the C# example was tested, which was systematically failed
without using this patch.

   mapObj m_obj = new mapObj(args[0]);	

   Console.WriteLine ("# Map layers " + m_obj.numlayers + "; Map name
= " + m_obj.name);

   for (int i=0; i<m_obj.numlayers; i++)

   {
     Console.WriteLine("Layer [" + i + "] name: " + m_obj.getLayer(i).name);

   }
   layerObj layer1 = m_obj.getLayer(1);
   layerObj layer2 = m_obj.getLayer(2);
   layerObj layer3 = m_obj.getLayer(3);
   layerObj layer4 = m_obj.getLayer(1);
   layerObj layer5 = m_obj.getLayer(2);
   layerObj layer6 = m_obj.getLayer(3);
   GC.Collect();
   classObj classo = layer5.getClass(0);



We should continue to deal with rewriting the arrays of structs to
arrays of pointers. Implementing Add/Remove on the collections of
classes cannot be done properly with the current solution. But this
problem can be handled separately.


Best regards,

Tamas
-------------- next part --------------

/******************************************************************************
 * $Id: swig_csharp_extensions.i 10639 2007-01-17 20:57:32Z tamas $
 *
 * Name:     swig_csharp_extensions.i
 * Project:  GDAL SWIG Interface
 * Purpose:  Fix for the SWIG Interface problems (early GC) 
 *           and implementing SWIGTYPE *DISOWN 
 * Author:   Tamas Szekeres
 *
*/

%typemap(csout, excode=SWIGEXCODE) SWIGTYPE {
    $&csclassname ret = new $&csclassname($imcall, null);$excode
    return ret;
  }
  
%define %implement_class(TYPE)
%typemap(csout, excode=SWIGEXCODE, new="1") TYPE & {
    $csclassname ret = new $csclassname($imcall, THISOWN_$owner());$excode
    return ret;
  }
%typemap(csout, excode=SWIGEXCODE, new="1") TYPE *, TYPE [], TYPE (CLASS::*) {
    IntPtr cPtr = $imcall;
    $csclassname ret = (cPtr == IntPtr.Zero) ? null : new $csclassname(cPtr, THISOWN_$owner());$excode
    return ret;
  }
%typemap(csvarout, excode=SWIGEXCODE2) TYPE & %{
    get {
      $csclassname ret = new $csclassname($imcall, THISOWN_$owner());$excode
      return ret;
    } %}
%typemap(csvarout, excode=SWIGEXCODE2) TYPE *, TYPE [], TYPE (CLASS::*) %{
    get {
      IntPtr cPtr = $imcall;
      $csclassname ret = (cPtr == IntPtr.Zero) ? null : new $csclassname(cPtr, THISOWN_$owner());$excode
      return ret;
    } %}
%typemap(csout, excode=SWIGEXCODE) TYPE *& {
    IntPtr cPtr = $imcall;
    $*csclassname ret = (cPtr == IntPtr.Zero) ? null : new $*csclassname(cPtr, THISOWN_$owner());$excode
    return ret;
  }
// Proxy classes (base classes, ie, not derived classes)
%typemap(csbody) TYPE %{
  private HandleRef swigCPtr;
  protected object swigCMemOwner;
  
  protected static object THISOWN_true() { return null; }
  protected object THISOWN_false() { return this; }

  internal $csclassname(IntPtr cPtr, object cMemoryOwner) {
    swigCMemOwner = cMemoryOwner;
    swigCPtr = new HandleRef(this, cPtr);
  }

  internal static HandleRef getCPtr($csclassname obj) {
    return (obj == null) ? new HandleRef(null, IntPtr.Zero) : obj.swigCPtr;
  }
  internal static HandleRef getCPtrAndDisown($csclassname obj, object cMemoryOwner) {
    obj.swigCMemOwner = cMemoryOwner;
    return getCPtr(obj);
  }
%}

// Derived proxy classes
%typemap(csbody_derived) TYPE %{
  private HandleRef swigCPtr;

  internal $csclassname(IntPtr cPtr, object cMemoryOwner) : base($modulePINVOKE.$csclassnameUpcast(cPtr), cMemoryOwner) {
    swigCPtr = new HandleRef(this, cPtr);
  }

  internal static HandleRef getCPtr($csclassname obj) {
    return (obj == null) ? new HandleRef(null, IntPtr.Zero) : obj.swigCPtr;
  }
  internal static HandleRef getCPtrAndDisown($csclassname obj, object cMemoryOwner) {
    obj.swigCMemOwner = cMemoryOwner;
    return getCPtr(obj);
  }
%}

// Typewrapper classes
%typemap(csbody) TYPE *, TYPE &, TYPE [], TYPE (CLASS::*) %{
  private HandleRef swigCPtr;

  internal $csclassname(IntPtr cPtr, object futureUse) {
    swigCPtr = new HandleRef(this, cPtr);
  }

  protected $csclassname() {
    swigCPtr = new HandleRef(null, IntPtr.Zero);
  }

  internal static HandleRef getCPtr($csclassname obj) {
    return (obj == null) ? new HandleRef(null, IntPtr.Zero) : obj.swigCPtr;
  }
%}

%typemap(csfinalize) TYPE %{
  ~$csclassname() {
    Dispose();
  }
%}

%typemap(csconstruct, excode=SWIGEXCODE) TYPE %{: this($imcall, null) {$excode
  }
%}

%typemap(csdestruct, methodname="Dispose", methodmodifiers="public") TYPE {
  lock(this) {
      if(swigCPtr.Handle != IntPtr.Zero && swigCMemOwner == null) {
        swigCMemOwner = new object();
        $imcall;
      }
      swigCPtr = new HandleRef(null, IntPtr.Zero);
      GC.SuppressFinalize(this);
    }
  }

%typemap(csdestruct_derived, methodname="Dispose", methodmodifiers="public") TYPE {
  lock(this) {
      if(swigCPtr.Handle != IntPtr.Zero && swigCMemOwner == null) {
        swigCMemOwner = new object();
        $imcall;
      }
      swigCPtr = new HandleRef(null, IntPtr.Zero);
      GC.SuppressFinalize(this);
      base.Dispose();
    }
  }
          
%enddef


%typemap(csin) SWIGTYPE *DISOWN "$csclassname.getCPtrAndDisown($csinput, this)"



/******************************************************************************
 * Mapscript objects with advanced memory handling 
 *****************************************************************************/

%implement_class(mapObj)
%implement_class(map_obj)
%implement_class(layerObj)
%implement_class(layer_obj)
%implement_class(classObj)
%implement_class(class_obj)
%implement_class(styleObj)
%implement_class(fontSetObj)
%implement_class(resultCacheObj)
%implement_class(resultCacheMemberObj)
%implement_class(labelCacheMemberObj)
%implement_class(labelCacheObj)
%implement_class(markerCacheMemberObj)
%implement_class(outputFormatObj)
%implement_class(symbolObj)
%implement_class(symbolSetObj)
%implement_class(webObj)
%implement_class(legendObj)
%implement_class(queryMapObj)
%implement_class(scalebarObj)
%implement_class(referenceMapObj)
%implement_class(legendObj)
%implement_class(imageObj)
%implement_class(labelObj)
%implement_class(rectObj)
%implement_class(shapeObj)
%implement_class(shapefileObj)
%implement_class(lineObj)
%implement_class(pointObj)
%implement_class(projectionObj)
%implement_class(hashTableObj)
%implement_class(colorObj)
  
/******************************************************************************
 * Preventing to take ownership of the memory when constructing objects 
 * with parent objects (causing nullreference exception, Bug 1743)
 *****************************************************************************/
    
%typemap(csconstruct, excode=SWIGEXCODE) layerObj(mapObj map) %{: this($imcall, map) {$excode
}
%}
%typemap(csconstruct, excode=SWIGEXCODE) classObj(layerObj layer) %{: this($imcall, layer) {$excode
}
%}
%typemap(csconstruct, excode=SWIGEXCODE) styleObj(classObj parent_class) %{: this($imcall, parent_class) {$excode
}
%}