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

[Chris Fields]

On Nov 24, 2008, at 10:59 PM, George Hartzell wrote:

>> ...
>>
>> In this case, when the last ref to the Bio::Species is released it
>> should be garbage collected and trigger DESTROY (which is set up in
>> Bio::Root::Root).  This in turn triggers any cleanup methods, such as
>> calling DESTROY on the Tree and Taxon and get rid of memory leaks  
>> (and
>> wouldn't require weaken/is_weak).
>> [...]
>
> How will this interact with the places that Bio::Species unhooks the
> various cleanup mechanisms that Tree sets up.  There are comments that
> talk about the sub ref screwing with someone's use of Storable.   It
> seems like if the new Species has cleanup code registered then there's
> a problem with whom-ever had a problem with it before.

http://bugzilla.open-bio.org/show_bug.cgi?id=2149#c1

The deleted root cleanup methods are for the internal  
Bio::Tree::Tree.  Apparently code refs don't persist using Storable or  
BioSQL (though I thought they could using B::Deparse?).

Anyway, for now I have everything set up in Bio::Species::DESTROY,  
which calls SUPER::DESTROY(), then the Tree and Taxon cleanup methods  
prior to explicitly deleting them.  I also removed all weaken/is_weak  
and the Scalar::Utils requirement from Bio::Species.  It passes all  
tests in the test suite.  Also, I didn't see any significant memory  
issues using Rutger's original file and the test file in the above bug  
report, but it's worth another run through with more data..

>> I'm perfectly willing to test that out if no one else is (I am  
>> working
>> on a few other bugs at the moment but I can probably get to it in the
>> next day or two).  Not to tread on any toes, but I would like to get
>> this one taken care of prior to 1.6 and these are fairly nasty memory
>> issues.  If there is any consolation it shouldn't be too hard to re-
>> rig the methods to delegate appropriately.
>>
>>> I don't really feel hot or bothered about this fix vs. the  
>>> combination
>>> of weaken and stashing the subspecies.  It depends on what the  
>>> author
>>> intended for Species and its tree, neither are particularly sexy and
>>> if Species is really on its way out the door it may not matter.....
>>>
>>> g.
>>
>> I don't think it will matter post-1.6, but we'll have to support
>> Bio::Species for the 1.6 release series.  So it will have to be
>> maintained for the short term, until 1.7 comes out.
>
> I can't poke at this stuff much until the weekend.  If you can kick
> it, go for it!
>
> g.

As you might have guessed I already have this set up.  If there aren't  
objections I'll commit the code tomorrow for testing.  We can role  
back if there are significant issues.

-c