Bug #125981 for Mojolicious-Plugin-Util-RandomString: memory leak with $mojo in closure

Wed Aug 01 17:44:29 2018 ether [...] cpan.org - Ticket created

Subject:

memory leak with $mojo in closure

I added Test::Memory::Cycle to a test suite to check for memory leaks on my app object, and it found this leak in your plugin: # Cycle #1 # Test::MyApp A->{ua} => Mojo::UserAgent B # Mojo::UserAgent B->{server} => Mojo::UserAgent::Server C # Mojo::UserAgent::Server C->{app} => MyApp D # MyApp D->{renderer} => Mojolicious::Renderer E # Mojolicious::Renderer E->{helpers} => %F # %F->{random_string} => &G # closure &G, $mojo => $H # $H => MyApp D It looks like $mojo should be weakened when it is closed over in the random_string sub. (See Scalar::Util::weaken.)

Sat Aug 04 03:34:07 2018 AKRON [...] cpan.org - Correspondence added

Am Mi 01. Aug 2018, 17:44:29, ETHER schrieb: Show quoted text

> I added Test::Memory::Cycle to a test suite to check for memory leaks > on my app object, and it found this leak in your plugin: > > # Cycle #1 > # Test::MyApp A->{ua} => Mojo::UserAgent B > # Mojo::UserAgent B->{server} => Mojo::UserAgent::Server C > # Mojo::UserAgent::Server C->{app} => MyApp D > # MyApp D->{renderer} => Mojolicious::Renderer E > # Mojolicious::Renderer E->{helpers} => %F > # %F->{random_string} => &G > # closure &G, $mojo => $H > # $H => MyApp D > > It looks like $mojo should be weakened when it is closed over in the > random_string sub. (See Scalar::Util::weaken.)

Thank you for finding this bug and even hinting for a solution! I will try to fix it next week.

Sat Aug 04 03:34:07 2018 The RT System itself - Status changed from 'new' to 'open'

Wed Aug 08 11:21:56 2018 ether [...] cpan.org - Correspondence added

My solution wasn't that great. :) Instead of weakening the $mojo object that was passed to you in the 'register' call, just get it at runtime from the arguments passed to the helper sub (the one you're shifting off and not using right now). I think it's a $c (controller), so instead of $mojo, just use $c->app. e.g. $c->app->log->warn(qq!RandomString generator "$gen" is unknown!);

Fri Aug 10 10:15:26 2018 AKRON [...] cpan.org - Correspondence added

Am Mi 08. Aug 2018, 11:21:56, ETHER schrieb: Show quoted text

> My solution wasn't that great. :) Instead of weakening the $mojo > object that was passed to you in the 'register' call, just get it at > runtime from the arguments passed to the helper sub (the one you're > shifting off and not using right now). I think it's a $c (controller), > so instead of $mojo, just use $c->app. > > e.g. $c->app->log->warn(qq!RandomString generator "$gen" is unknown!);

Today I was able to check, test and hopefully fix the issue in the 0.07 release. Thank you very, very much for the bug report and introducing me to Test::Memory::Cycle! I wasn't aware of this extremely useful tool!

Fri Aug 10 10:15:27 2018 AKRON [...] cpan.org - Status changed from 'open' to 'patched'

Fri Aug 10 10:15:27 2018 AKRON [...] cpan.org - Taken

Fri Aug 10 10:15:27 2018 AKRON [...] cpan.org - Fixed in 0.07 added

Sat Aug 11 17:52:02 2018 ether [...] cpan.org - Correspondence added

On 2018-08-10 07:15:26, AKRON wrote: Show quoted text

> Today I was able to check, test and hopefully fix the issue in the > 0.07 release. Thank you very, very much for the bug report and > introducing me to Test::Memory::Cycle! I wasn't aware of this > extremely useful tool!

Awesome, thank you! And thanks for that other thing!

Wed May 15 05:27:55 2019 AKRON [...] cpan.org - Correspondence added

Am Sa 11. Aug 2018, 17:52:02, ETHER schrieb: Show quoted text

> On 2018-08-10 07:15:26, AKRON wrote: >

> > Today I was able to check, test and hopefully fix the issue in the > > 0.07 release. Thank you very, very much for the bug report and > > introducing me to Test::Memory::Cycle! I wasn't aware of this > > extremely useful tool!

> > > Awesome, thank you! And thanks for that other thing!

Wed May 15 05:27:56 2019 AKRON [...] cpan.org - Status changed from 'patched' to 'resolved'