Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leaks #3

Closed
GoogleCodeExporter opened this issue Nov 28, 2015 · 4 comments
Closed

Memory leaks #3

GoogleCodeExporter opened this issue Nov 28, 2015 · 4 comments

Comments

@GoogleCodeExporter
Copy link

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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

  • Changed state: Accepted

@GoogleCodeExporter
Copy link
Author

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

  • Changed state: Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant