Skip Menu |
 

This queue is for tickets about the XML-Twig CPAN distribution.

Report information
The Basics
Id: 56838
Status: resolved
Priority: 0/
Queue: XML-Twig

People
Owner: Nobody in particular
Requestors: andrew [...] pimlott.net
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: 3.35



CC: Andrew Pimlott <andrew [...] pimlott.net>
Subject: reference loop through twig_ignore_action
Date: Thu, 22 Apr 2010 17:35:29 -0700
To: bug-XML-Twig [...] rt.cpan.org
From: Andrew Pimlott <andrew [...] pimlott.net>
Download (untitled) / with headers
text/plain 417b
If you run the following code under XML::Twig 3.35, my $twig = XML::Twig->new( ignore_elts => { x => 1 } ); $twig->parse("<x/>"); $twig->dispose(); print "$twig $twig->{twig_ignore_action}\n"; you will find that $twig->{twig_ignore_action} points back to $twig. Thus, $twig will never get garbage collected. The line my $action= shift || 1; in the ignore function at line 3803 looks highly suspect. Andrew
Subject: Re: [rt.cpan.org #56838] AutoReply: reference loop through twig_ignore_action
Date: Thu, 22 Apr 2010 18:12:11 -0700
To: bug-XML-Twig <bug-xml-twig [...] rt.cpan.org>
From: Andrew Pimlott <andrew [...] pimlott.net>
Download (untitled) / with headers
text/plain 461b
Follow-up suggestion: The reason this bug was so painful to me was not the memory leak; it was that because the twig stayed alive, my handlers stayed alive, and the handlers had references to objects that needed to destruct after parsing finished. I propose that ->dispose unset the twig's handlers. That way, even if the twig itself doesn't go away (which would of course be a bug, but bugs happen), at least my handlers would be garbage collected. Andrew
Subject: Re: [rt.cpan.org #56838] reference loop through twig_ignore_action
Date: Fri, 23 Apr 2010 11:53:13 -0600
To: bug-XML-Twig [...] rt.cpan.org
From: Michel Rodriguez <xmltwig [...] gmail.com>
Download (untitled) / with headers
text/plain 330b
I think you're right, the line $action= shift || 1 should be $action= $_[2] || 1, @_ has not been changed at that point, so its first element is the twig itself. I have to write the tests. As for unsetting the handlers in dispose, yes it can be done. I'll do this probably early next week. Thanks for the bug report. -- mirod
Subject: Re: [rt.cpan.org #56838] reference loop through twig_ignore_action
Date: Fri, 23 Apr 2010 13:39:58 -0700
To: bug-XML-Twig <bug-xml-twig [...] rt.cpan.org>
From: Andrew Pimlott <andrew [...] pimlott.net>
Download (untitled) / with headers
text/plain 509b
Excerpts from xmltwig@gmail.com via RT's message of Fri Apr 23 10:53:26 -0700 2010: Show quoted text
> I think you're right, the line $action= shift || 1 should be $action= > $_[2] || 1, @_ has not been changed at that point, so its first > element is the twig itself. I have to write the tests. As for > unsetting the handlers in dispose, yes it can be done.
Awesome, it's really nice to see you are still committed to this code! For now, I'm using the obvious work-around: $twig->{twig_ignore_action} = undef; Andrew
Subject: Re: [rt.cpan.org #56838] reference loop through twig_ignore_action
Date: Tue, 04 May 2010 14:09:00 -0700
To: bug-XML-Twig <bug-xml-twig [...] rt.cpan.org>
From: Andrew Pimlott <andrew [...] pimlott.net>
I just happened to look at the Twig DESTROY function. It looks like there is an attempt to completely clear out the Twig object: $t={}; # prevents memory leaks (especially when using mod_perl) undef $t; It it had worked, my memory leak would not have happened. However, this code doesn't actually do anything useful. It points $t to some new hash, then unsets it. This has no effect on the Twig object. For that, you probably want to empty the hash representing the Twig: %$t = (); If you do that (and it seems like a good idea to me), you can remove all the undef lines as they will be redundant. Andrew Excerpts from xmltwig@gmail.com via RT's message of Fri Apr 23 10:53:26 -0700 2010: Show quoted text
> <URL: http://rt.cpan.org/Ticket/Display.html?id=56838 > > > I think you're right, the line $action= shift || 1 should be $action= > $_[2] || 1, @_ has not been changed at that point, so its first > element is the twig itself. I have to write the tests. As for > unsetting the handlers in dispose, yes it can be done. > > I'll do this probably early next week. > > Thanks for the bug report. >
On Tue May 04 17:09:14 2010, andrew@pimlott.net wrote: Show quoted text
> I just happened to look at the Twig DESTROY function. It looks like > there is an attempt to completely clear out the Twig object: > > $t={}; # prevents memory leaks (especially when using mod_perl) > undef $t; > > It it had worked, my memory leak would not have happened. However, > this > code doesn't actually do anything useful. It points $t to some new > hash, then unsets it. This has no effect on the Twig object. For > that, > you probably want to empty the hash representing the Twig: > > %$t = (); > > If you do that (and it seems like a good idea to me), you can remove > all > the undef lines as they will be redundant.
I did %$t = (); although I am not quite certain that really helps. That's in XML::Twig 3.35 I would need more time to investigate potential memory leaks due to handlers not being freed, but I don't have much at the moment. Let me know if the new version fixes the problem for you, and if not I'll have a look at it. Thanks __ mirod
Subject: Re: [rt.cpan.org #56838] reference loop through twig_ignore_action
Date: Mon, 07 Jun 2010 11:56:24 -0700
To: bug-XML-Twig <bug-xml-twig [...] rt.cpan.org>
From: Andrew Pimlott <andrew [...] pimlott.net>
Download (untitled) / with headers
text/plain 1.3k
Sorry not to follow up sooner. I'm working with XML::Twig again, and I tested your changes in the latest 3.35, and they do fix things for me. Thanks! Andrew Excerpts from MIROD via RT's message of Tue May 18 02:57:41 -0700 2010: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=56838 > > > On Tue May 04 17:09:14 2010, andrew@pimlott.net wrote:
> > I just happened to look at the Twig DESTROY function. It looks like > > there is an attempt to completely clear out the Twig object: > > > > $t={}; # prevents memory leaks (especially when using mod_perl) > > undef $t; > > > > It it had worked, my memory leak would not have happened. However, > > this > > code doesn't actually do anything useful. It points $t to some new > > hash, then unsets it. This has no effect on the Twig object. For > > that, > > you probably want to empty the hash representing the Twig: > > > > %$t = (); > > > > If you do that (and it seems like a good idea to me), you can remove > > all > > the undef lines as they will be redundant.
> > I did %$t = (); although I am not quite certain that really helps. > That's in XML::Twig 3.35 > > I would need more time to investigate potential memory leaks due to > handlers not being freed, but I don't have much at the moment. Let me > know if the new version fixes the problem for you, and if not I'll have > a look at it. > > Thanks > __ > mirod


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.