[Bioperl-l] A better fix for Species.pm memory leak, please REVIEW.

[Chris Fields]

On Nov 22, 2008, at 6:21 PM, George Hartzell wrote:

> I dug back into the Species.pm memory leak and my less-than-ideal fix
> for it.
>
> First, in my defense, my previous fix only causes failures in
> t/Species.t if network tests are enabled.  It passes the generic
> ones.  I *thought* I'd run the tests before I committed :).
>
> The basic cause of the problem is that when species() calls
> Bio::Tree::Tree->new(-node=> $species_taxon), $species_taxon is a copy
> of $self.  The tree ends up adding that node onto the classification
> list and it gets linked it into the descendent links, which results in
> a cycle.

...

> If that lousy english description left you scratching your head,
> here's a hacked-to-pieces version of R Voss's original test case.
> You'll need Devel::Cycle (YAY LINCOLN!) to run it.  Find yourself a
> genbank file (I used gbpri21.seq, from the original bug report), put
> it into a directory somewhere and do something like
>
> ...
> Anyway.  Here's a fix.  Instead of adding $self to the tree, make a
> new species node w/ $self's important bits and add that instead.
> There's a patch below that gets rid of my weaken and Sendu's
> fix-for-the-fix and then Does The Right Thing (I hope).
>
> It seems to pass all of the Species.t tests.
>
> The take home exam questions are:  Are there any other missing
> important bits?

The only thing I can suggest is to make sure it deals with the  
original problem (memory leaks, per bug 2594):

http://bugzilla.open-bio.org/show_bug.cgi?id=2594

> Thoughts?

I'm not quite sure why we are attaching a Bio::Tree::Tree to the  
Bio::Species, so I need to go back and review the changes from the  
1.5.2 release (I'm sure there is a good reason, just can't recall and  
haven't had time to look).

However, we aren't planning on retaining Bio::Species too much longer  
anyway (we'll be moving towards a simpler Bio::Taxon-based system  
after the 1.6 release and will be deprecating Bio::Species  
eventually).  Any fixes should probably take this into consideration.

> I'll commit it if someone'll review it.
>
> g.

I vote to go ahead and commit this (the solution seems sound), but  
Sendu is the one who would be best to vet as he refactored this.  If  
there isn't word by the middle of the week you can go ahead and make  
the commit.  Thanks George!

chris