develooper Front page | perl.perl5.porters | Postings from September 2010

Re: [perl #75176] Symbol::delete_package does not free certain memory associated with package::ISA

Thread Previous | Thread Next
Father Chrysostomos
September 5, 2010 13:17
Re: [perl #75176] Symbol::delete_package does not free certain memory associated with package::ISA
Message ID:

On Aug 29, 2010, at 2:09 PM, Father Chrysostomos wrote:

> On Aug 22, 2010, at 5:46 PM, Father Chrysostomos wrote:
>> Initially I planned to fix the isarev leaks by making appropriate adjustments to it whenever an @ISA is modified or a stash is freed. But, since stashes can be detached from the main tree of stashes ($stash = delete $::{"it::"}), they need to know whether they are still attached, so detached stashes don’t muddle up isarev and delete entries that are still needed (which would be worse than leaking).
>> In the process of experimenting with that, I found that such stash manipulations already fail to update isa caches, which, too, would cause my proposed isarev changes to introduce regressions. So that’s what this patch is for.
> Here is a patch that deals with a couple more ways of manipulating stashes.
> I plan to work on assignment to empty stash elements next (or any stash elements that are not globs). I can do this by using PVLVs. But there are two approaches:
> 1. Do it just for access from Perl by adding it to pp_helem.
> 2. Do it for XS access as well, by modifying hv_fetch_ent instead (or hv_common, or wherever the actual code is).
> Number 1 makes it too easy for XS code to make changes without triggering mro_package_moved. mro_package_moved will also have to be made public (and probably given a better name).
> Number 2 is probably more reliable. perl’s internals can pass a flag to avoid the PVLVs. XS code that assumes that stash elements are never magical will be broken (code that specifically calls the _nomg forms to avoid the magic checks). Most XS code will continue to work.
> So in both cases, XS code will have to be modified (if there is any that plays with stashes). But in case 2 only code that is currently potentially buggy will have to change.
> I prefer number 2.

Here is a patch for this. I used approach number 1 because number 2 proved to be far more complicated than I had thought, and probably too risky. I am not so sure now that mro_package_moved needs to be made public. But we could change that later if necessary.

I did not bother with a mathom for mro_isa_changed_in, since this patch relies on the fix for #77362, which changes behaviour in a way that is not suitable for maint. (There are many other bugs that can occur more often as a result of that fix.)

Thread Previous | Thread Next Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About