[Insight-developers] Memory leaks in Mesh

[Miller, James V (Research)]

I don't think we want to remove any of this functionality. I just think
we need to fix the items that are currently broken and perhaps reduce the
complexity a bit.  One question that is readily apparent is that because
of the number of internal data structures in the mesh, it is very difficult
to determine who owns what memory.  There are vectors of raw points, 
there are vectors of SmartPointers, and then there are AutoPointers running 
around. So it is really unclear who is responsible for releasing memory.

For instance, if I create a cell and pass it to the mesh.  Do I release the 
memory for the cell or does the mesh ultimately release the memory.  If I
make a query on the mesh that returns some sort of "pointer", do I release the
memory or does the mesh.  I don't recall how these things operated when 
SmartPointers were used but the mixture of raw pointers and AutoPointers
currently used make it hard to decipher.

It would be nice to have this "resolved" before we 
cut the release.

For the problem associated with GetCellBoundaryFeature()
I am think of the following options:

If the boundary feature is an implicit feature, then 

1) create an explicit feature to add to the AssignedCellBoundary vector

2) create an explicit feature and add it to an "ImplicitAssignedCellBoundary"
vector

3) have different a flag in the features that indicate whether they are
explicit features of implicit features. I suppose this could be done with
a flag or a type (subclass).

option #2 would allow the mesh to keep track of what boundary features
were assigned by the user and what boundary features were created on 
the fly. (option #3 may allow the same thing)

Another thought that would simplify things is for the Mesh::SetBoundaryAssignment
to not take a BoundaryIdentifier.  It looks like the boundary assignments are held 
in a map that maps

<CellIdentifier, FeatureIdentifier> to a BoundaryIdentifier

So to make a boundary assignment, the user must know the cell they want to 
assign it to, the feature they want to assign it to, the dimension of the feature
they want to assign it to AND they must know where they want that assignment to 
occur in the boundary feature vector. I have no problem with the (cell, feature,
dimension) being needed.  But it may simplify things if the user did not have
to specify the boundary identifier.

If someone wants a boundary feature, they query the assignment map
to see if there is a BoundaryIdentifier associated with
the <CellIdentifer, FeatureIdentifier> pair and then they look up
the boundary feature using the returned BoundaryIdentifier in the 
boundary feature vector.

Instead of using a map and a vector, we could just a map that directly
converted the pair

<CellIdentifier, FeatureIdentifier> to a BoundaryFeature 







> -----Original Message-----
> From: Luis Ibanez [mailto:luis.ibanez@kitware.com]
> Sent: Tuesday, October 01, 2002 1:57 PM
> To: Miller, James V (Research)
> Cc: Insight-developers (E-mail)
> Subject: Re: [Insight-developers] Memory leaks in Mesh
> 
> 
> 
> Hi Jim,
> 
> It looks like the Mesh requires a major refactoring.
> 
> There is a large set of functionalities provided by
> the Mesh that don't seem to be used by the toolkit
> but cost a lot on the implementation.  For example
> the management of neighborhood relationships
> between cells and the policy of simulating explicit
> cells. Most of the dificulties in memory managment
> come from these features.
> 
> How about having a discussion on the use/design
> of the Mesh at next week meeting. Probably on
> Tuesday  ?
> 
> 
>    Luis
> 
> 
> ======================================
> 
> Miller, James V (Research) wrote:
> 
> > I am trying to track down the memory leaks in the mesh class. 
> >
> >  
> >
> > It looks like Mesh::SetCell(CellIdentifier, CellAutoPointer&) leaks 
> > memory if there was already a cell assigned at the specified 
> > CellIdentifier location.  The cells are ultimately kept in 
> a vector of 
> > pointers to cells (not SmartPointers, not AutoPointers).  So, 
> > SetCell() just overwrites the location in the vector with a new 
> > pointer to a cell.
> >
> >  
> >
> > This means the user of the Mesh has to manage the 
> de-allocation of the 
> > cell currently at the CellIdentifier position prior to calling 
> > SetCell().  This should be documented somewhere. I modified the 
> > itkMeshTest to do this deallocation.
> >
> >  
> >
> > It also looks like Mesh::GetCellBoundaryFeature() leaks when it is 
> > asked for the same "implicit" boundary feature multiple 
> times.  When 
> > asked for an implicit boundary feature, memory get 
> allocated to hold 
> > the feature (i.e. when asking for an edge of a triangle where the 
> > triangle is just specified with 3 vertices but no explicit 
> edges are 
> > defined).  This created feature gets inserted into the vector of 
> > boundary features.  When called a second time for the same implicit 
> > boundary feature, memory gets allocated for the feature and the 
> > feature overwrites the previously created boundary feature.  This 
> > causes a leak.  There is a check in the code that if the 
> cell boundary 
> > feature is one of the "assigned" boundary features (explicit 
> > features), then it simply returns the feature from the boundary 
> > feature vector.  However, when a boundary feature is implicit, an 
> > object is created to be returned to the user and that 
> object is added 
> > to the vector of boundary features but that object is NOT 
> added to the 
> > list of ASSIGNED boundary features.  So the check in the code fails 
> > and a new feature is allocated and overwrites the previous boundary 
> > feature pointer in the boundary feature vector. Once an implicit 
> > feature is queried, should it be converted (and recorded) as an 
> > explicit and assigned feature?
> >
> >  
> >
> > * Jim Miller*
> > */_____________________________________/*
> > / Visualization & Computer Vision//
> > /GE Research/
> > /Bldg. KW, Room C218B/
> > /P.O. Box 8, Schenectady NY 12301/
> >
> > //_ millerjv@research.ge.com <mailto:millerjv@research.ge.com> _/
> >
> > /_ james.miller@research.ge.com_/
> > /(518) 387-4005, Dial Comm: 8*833-4005, /
> > /Cell: (518) 505-7065, Fax: (518) 387-6981/
> >
> >  
> >
> >  
> >
> 
> 
>