agusw1 / poly2tri

Automatically exported from code.google.com/p/poly2tri
Other
0 stars 0 forks source link

Memory leaks #3

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Hello there,

First of all thanks for this nice small lib.
I've checked (the C++) version for memory leaks with valgrind and I think
you probably have some memory leaks in the following methods:

   * p2t::Sweep::NewFrontTriangle
   * p2t::SweepContext::InitEdges
   * p2t::SweepContext::CreateAdvancingFront (most important one)

Original issue reported on code.google.com by Dimitris...@gmail.com on 12 Mar 2010 at 6:35

GoogleCodeExporter commented 9 years ago
Thanks for finding the leaks! Care to submit a patch?

Original comment by mason.gr...@gmail.com on 13 Mar 2010 at 2:46

GoogleCodeExporter commented 9 years ago
The CreateAdvancingFront leak seems to be a matter of dealing with this TODO in
Sweep::Fill() at sweep/sweep.cc line 210:

    -  // TODO: delete node from memory
    -  //tcx.RemoveNode(node);
    +  tcx.RemoveNode(&node);

And then we have to avoid using the freed memory in Sweep::FillAdvancingFront() 
line
230 and similarly at line 240.  Partial workaround:

    -    Fill(tcx, *node);
    -    node = node->next;
    +    Node *tmp = node;
    +    node = node->next;
    +    Fill(tcx, *tmp);

That works with 'dude.dat' but segfaults with some other tests like 'bird.dat'. 
There are other pointers to the node being deleted... I don't have time to sort 
it
all out now.

Original comment by tnove...@gmail.com on 9 Apr 2010 at 3:27

GoogleCodeExporter commented 9 years ago
The problem stems from NOT deleting nodes from memory after they are discarded 
from 
the advancing front. I know how to fix; it's just a matter of finding the time.

Original comment by mason.gr...@gmail.com on 9 Apr 2010 at 3:31

GoogleCodeExporter commented 9 years ago
Fixed memory leaks by manually deleting nodes in obvious places when they're no 
longer 
needed. There were still a few strays that I did not track down, although I 
solved 
this problem by simply stuffing all new nodes into a vector container. If 
there's ever 
a need for a non-STL version someone will need to spend a little extra time and 
fix. 

Original comment by mason.gr...@gmail.com on 23 Apr 2010 at 5:24