Skip Menu |

This queue is for tickets about the WWW-Mechanize-Chrome CPAN distribution.

Report information
The Basics
Id: 129808
Status: resolved
Priority: 0/
Queue: WWW-Mechanize-Chrome

People
Owner: Nobody in particular
Requestors: fsologuren [...] newtenberg.com
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 0.32
Fixed in: (no value)



Subject: The listener handler wont be removed when its reference is discarded
Hello, Because the add_listener method of the driver store the Chrome::DevToolsProtocol::EventListener instance in an array (https://metacpan.org/source/CORION/WWW-Mechanize-Chrome-0.32/lib/Chrome/DevToolsProtocol.pm#L225), the dereference action of this instance don't leave it available to the garbage collector, so the listener instance never execute his DESTROY method, remaining for ever in the array, consuming a crecient amount of memory and multiplying this listener actions. Attached goes a litle concept proof of the effect. Thank you.
Subject: leak_test.pl
use WWW::Mechanize::Chrome; my $mech = WWW::Mechanize::Chrome->new(); my $res = $mech->get('https://www.google.com'); do { my @input = $mech->xpath('//input[@name="q"]'); print 'scalar @{ $mech->driver->listener->{\'DOM.setChildNodes\'} }='. scalar @{ $mech->driver->listener->{'DOM.setChildNodes'} } ."\n"; sleep 1; } while ( scalar @{ $mech->driver->listener->{'DOM.setChildNodes'} } < 10 );
El Mié Jun 12 23:37:00 2019, fsologuren@newtenberg.com escribió: Show quoted text
> Hello, > > Because the add_listener method of the driver store the > Chrome::DevToolsProtocol::EventListener instance in an array > (https://metacpan.org/source/CORION/WWW-Mechanize-Chrome- > 0.32/lib/Chrome/DevToolsProtocol.pm#L225), the dereference action of > this instance don't leave it available to the garbage collector, so > the listener instance never execute his DESTROY method, remaining for > ever in the array, consuming a crecient amount of memory and > multiplying this listener actions.
The method remove_listener needs an 'event' property in the listener handler https://metacpan.org/source/CORION/WWW-Mechanize-Chrome-0.32/lib/Chrome/DevToolsProtocol.pm#L239, but class Chrome::DevToolsProtocol::EventListener has not that property since "Move to Moo" commit. Attached goes two suggested patches. Thank you.
Subject: Chrome.pm.listener.patch
--- /usr/share/perl5/WWW/Mechanize/Chrome.pm~ 2019-06-13 11:25:42.747207787 -0400 +++ /usr/share/perl5/WWW/Mechanize/Chrome.pm 2019-06-13 19:50:52.162083951 -0400 @@ -3165,6 +3165,7 @@ # ); #} )->then( sub( $response ) { + $setChildNodes->unregister(); undef $setChildNodes; my %nodes = map { $_->{nodeId} => $_
Subject: DevToolsProtocol.pm.patch
--- /usr/share/perl5/Chrome/DevToolsProtocol.pm~ 2019-05-17 08:34:40.000000000 -0400 +++ /usr/share/perl5/Chrome/DevToolsProtocol.pm 2019-06-13 19:48:59.926056441 -0400 @@ -778,7 +778,12 @@ is => 'ro', ); +has 'event' => ( + is => 'ro', +); + around BUILDARGS => sub( $orig, $class, %args ) { + croak "Need an event" unless $args{ event }; croak "Need a callback" unless $args{ callback }; croak "Need a DevToolsProtocol in protocol" unless $args{ protocol }; return $class->$orig( %args )
Thank you very much for the diagnosis and patches! The changes have been released in 0.33.
El Vie Jun 14 02:47:24 2019, CORION escribió: Show quoted text
> Thank you very much for the diagnosis and patches! > > The changes have been released in 0.33.
Thanks to you! I think that another registration cases needs the same remove treatment: Here: https://metacpan.org/source/CORION/WWW-Mechanize-Chrome-0.33/lib/WWW/Mechanize/Chrome.pm#L1071, here: https://metacpan.org/source/CORION/WWW-Mechanize-Chrome-0.33/lib/WWW/Mechanize/Chrome.pm#L1129, and here: https://metacpan.org/source/CORION/WWW-Mechanize-Chrome-0.33/lib/WWW/Mechanize/Chrome.pm#L5049, do you need the patches? In these three cases https://metacpan.org/source/CORION/WWW-Mechanize-Chrome-0.33/lib/WWW/Mechanize/Chrome.pm#L804 I'm not sure. On the other hand, I think that the user of the mech needs a remove_listener method to can remove their own handler (complementing the add_listener from WWW-Mechanize-Chrome). Thank you again!
Subject: Re: [rt.cpan.org #129808] The listener handler wont be removed when its reference is discarded
Date: Fri, 14 Jun 2019 23:01:38 +0200
To: bug-WWW-Mechanize-Chrome [...] rt.cpan.org
From: Max Maischein <corion [...] corion.net>
Hello Felipe, Show quoted text
>> The changes have been released in 0.33.> Thanks to you!
You're welcome! It's always great to have users for a module, and even better if they provide solutions for the problems they find with my code :) Show quoted text
No - I've added tests that the destructors get called when the listeners are removed. Show quoted text
> In these three cases https://metacpan.org/source/CORION/WWW-Mechanize-Chrome-0.33/lib/WWW/Mechanize/Chrome.pm#L804 I'm not sure. > > On the other hand, I think that the user of the mech needs a remove_listener method to can remove their own handler (complementing the add_listener from WWW-Mechanize-Chrome).
You can remove your listener by setting your listener variable to undef. But I'll add the "remove_listener" method to the event listener , for the cases where you want to make things clearer, that is still a good idea! -max