The reason to call the original DESTROY is so that if a future version of
IO::All adds logic to the DESTROY, you will still run that new code. While
still running the code that you know you need to run.
In fact I would suggest the following instead:
use IO::All;
{
no warnings 'redefine';
my $orig_destroy = *IO::All::DESTROY{CODE};
sub IO::All::DESTROY {
my $self = shift;
no warnings;
untie *$self if tied *$self;
$orig_destroy->($self, @_) if $orig_destroy;
}
}
Now the custom code that you run is limited to what you are afraid that
their DESTROY gets wrong.
(Note that my original version had a bug. I said IO::ALL::DESTROY when it
needed to be IO::All::DESTROY.)
On Tue, Dec 13, 2016 at 1:11 PM, Duane Bronson <[email protected]>
wrote:
> The original DESTROY had the leaky code (see the commented #unless below),
> so I don't think it would help to keep calling it. My replacement DESTROY
> is a copy/paste of the one in IO/All.pm with the bad lines commented out.
>
> I haven't really been able to find the implementation of the version
> class. I hoped it was just missing a destructor that I could copy from a
> newer perl and conditionally add it to the package.
>
> Yes, our product is using scientific linux 6.3 which uses perl 5.10.x, and
> our customers and QA don't like hot fixes that replace nearly every RPM.
> In fact, they would probably prefer the memory leak.
>
> Duane
>
> On Dec 13, 2016, at 2:03 PM, Ben Tilly <[email protected]> wrote:
>
> There is always a risk when working with the internals of another module.
> I would minimize that risk by making sure that the DESTROY that you are
> replacing always runs. Just in case something gets added to it. And
> capture the reference to it in a way that will notice if DESTROY is
> eliminated.
>
> Like this:
>
> use IO::All;
> {
> no warnings 'redefine';
> my $orig_destroy = *IO::ALL::DESTROY{CODE};
> sub IO::All::DESTROY {
> my $self = shift;
> $orig_destroy->($self, @_) if $orig_destroy;
> no warnings;
> untie *$self if tied *$self;
> $self->close if $self->is_open;
> }
> }
>
> On Tue, Dec 13, 2016 at 10:04 AM, Duane Bronson <[email protected]>
> wrote:
>
>> Mongers,
>>
>> I've been trying to track down a memory leak in a long running perl
>> script that uses IO::All to write a bunch of files every few seconds. I
>> traced my leak back to IO::All which checks to make sure the version is at
>> least 5.8.0 before calling untie. Pretty innocuous, imho.
>>
>> Here's an easy way to reproduce it. In perl 5.10.1, this script causes
>> perl's memory to grow really big. It appears to be fixed in later versions
>> of perl because there is no memory leak on my mac (perl 5.18.2).
>>
>> perl -E 'for $i (0..100_000_000) { 1 if ($^V) }'
>> and
>> perl -E '$foo = version->new(v1.2.3); for $i (0..100_000_000) { 1 if
>> ($foo) }'
>> and (slower)
>> perl -E 'use IO::All; for $i (0..100_000_000) { "file contents" >
>> io("/tmp/filename") }'
>>
>> Is there a way of fixing my script so I can still use IO::All and not
>> have a memory leak? Here is one way, but I suspect there are better
>> methods that won't break with a future version of IO::All.
>>
>> use IO::All;
>> {
>> no warnings 'redefine';
>> sub IO::All::DESTROY {
>> my $self = shift;
>> no warnings;
>> #unless ( $^V and $^V lt v5.8.0 ) {
>> untie *$self if tied *$self;
>> #}
>> $self->close if $self->is_open;
>> }
>> }
>>
>> Thanks,
>> Duane
>>
>>
>>
>> Duane Bronson
>> [email protected] <mailto:[email protected]>
>> http://www.nerdlogic.com/ <http://www.nerdlogic.com/>
>> 5 Goden St.
>> Belmont, MA 02478
>> 617.515.2909
>>
>>
>>
>>
>>
>> _______________________________________________
>> Boston-pm mailing list
>> [email protected]
>> http://mail.pm.org/mailman/listinfo/boston-pm
>
>
>
>
> *Duane Bronson*
> [email protected]
> http://www.nerdlogic.com/
> 5 Goden St.
> Belmont, MA 02478
> 617.515.2909
>
>
>
>
>
_______________________________________________
Boston-pm mailing list
[email protected]
http://mail.pm.org/mailman/listinfo/boston-pm
