[pm-h] Adventures In Getting Bitten By a Closure - Is there a better way to address this issue?
G. Wade Johnson
gwadej at anomaly.org
Sun Dec 18 08:57:16 PST 2016
Unfortunately, Perl did exactly what you told it to do. Closures do make
a new reference to the object that does not exactly go away in the case
you are showing.
You should be able to solve the problem differently with
Scalar::Util::weaken, which explicitly reduces the reference count. The
only advantage of weaken is the fact that it is explicit and might not
be subject to later "correction" of something that seems redundant.
G. Wade
On Fri, 16 Dec 2016 16:51:46 -0600
Robert Stone via Houston <houston at pm.org> wrote:
> Greetings,
>
> Tried in 5.16.3 and 5.24.0. One of our debugging steps was to take
> advantage of perlbrew and try it in a different version of perl. Same
> result.
>
> Interesting idea that this could be a perl bug. I don't know if I'd
> call this a bug though, as much as perl doing *exactly* what you
> asked it to without knowing what you were asking.
>
> Best Regards,
> Robert Stone
>
> On Fri, Dec 16, 2016 at 4:40 PM, Julian Brown via Houston
> <houston at pm.org> wrote:
>
> > First of I love Test::MockModule,
> >
> > I am wondering if you run this on different versions of Perl if your
> > results would be different and possibly a bug in Perl for that
> > version?
> >
> > Julian
> >
> > On Fri, Dec 16, 2016 at 12:43 PM, Robert Stone via Houston
> > <houston at pm.org
> > > wrote:
> >
> >> Greetings,
> >>
> >>
> >> tl;dr - refcount unexpectedly incremented, caused DESTROY method on
> >> object to never get called. Used a workaround to address but
> >> perhaps there is a cleaner way?
> >>
> >>
> >> Jocelyn and I just spent about 3 hours tracking down why a DESTROY
> >> wasn't being called, a whole bunch of refcounts later we found the
> >> problem. I'm only fairly confident in my analysis below and would
> >> love it for any of you to chime in and say "Yep! Sounds about
> >> right" or "No way, what actually happened was..." Should be good
> >> times for all of us.
> >>
> >> For those of you who aren't fully versed on the magic that is
> >> Test::MockModule, it's the greatest thing since the circumflexion
> >> operator. It allows you to mock a method of an object and then
> >> when the variable that you assigned Test::MockModule->new to goes
> >> out of scope then the mock is removed. In general, very handy and
> >> it works perfectly.
> >>
> >> Here is the subroutine I was using to define and install my mock:
> >>
> >> sub restrict_search_space {
> >> my ( $user_ids ) = pos_validated_list( \@_, { isa =>
> >> 'ArrayRef[Int]' } );
> >>
> >> my $mocked_user = Test::MockModule->new(
> >> 'FreeChat::API::Model::User' );
> >> $mocked_user->mock(
> >> 'search',
> >> sub {
> >> my $self = shift;
> >> my $condition = shift;
> >> my $attributes = shift // {};
> >>
> >> $condition->{'me.id'} = { 'in' => $user_ids };
> >> return $mocked_user->original( 'search' )->( $self,
> >> $condition, $attributes );
> >> } );
> >>
> >> return $mocked_user;
> >> }
> >>
> >> Seems easy enough right? I create an instance of Test::MockModule
> >> that mocks FreeChat::API::Model::User. Then I mock the search
> >> method so that I can restrict the search space by adding an
> >> additional condition to the WHERE clause that is ultimately passed
> >> down to DBIx's search method. Because what I really want here is
> >> an AROUND instead of a straight up mock I end up having to call
> >> $mocked_user->original('search') to get the original unmocked
> >> method, thereby making this into a sort of AROUND.
> >>
> >> This works no problem until you try to do something like this
> >> (actual tests! Names were not changed to protect the guilty):
> >>
> >> subtest 'Gender' => sub {
> >> my ( $mech, $user ) = get_mech_and_user();
> >>
> >> my $male_user = create_user( gender => 'Male' );
> >> my $female_user = create_user( gender => 'Female' );
> >>
> >> my $mocked_user =
> >> restrict_search_space( [ $male_user->id, $female_user->id ] );
> >>
> >> $mech->get_ok( '/users?gender=Male' );
> >>
> >> note( 'Raw JSON Response: ' . $mech->content );
> >> my $json = from_json( $mech->content );
> >>
> >> ok( ( grep { $_->{id} == $male_user->id } @{$json} ),
> >> 'Matching user correctly identified' );
> >> ok( !( grep { $_->{id} == $female_user->id } @{$json} ),
> >> 'Unmatching user correctly not identified' );
> >> };
> >>
> >> subtest 'Status' => sub {
> >> my ( $mech, $user ) = get_mech_and_user();
> >>
> >> my $active_user = create_user( status => 'Active' );
> >> my $offline_user = create_user( status => 'Offline' );
> >>
> >> my $mocked_user =
> >> restrict_search_space( [ $active_user->id, $offline_user->id ] );
> >>
> >> $mech->get_ok( '/users?status=Active' );
> >>
> >> note( 'Raw JSON Response: ' . $mech->content );
> >> my $json = from_json( $mech->content );
> >>
> >> cmp_ok( $mech->status, '==', 200, 'Correct status' );
> >>
> >> ok( ( grep { $_->{id} == $active_user->id } @{$json} ),
> >> 'Matching user correctly identified' );
> >> ok( !( grep { $_->{id} == $offline_user->id } @{$json} ),
> >> 'Unmatching user correctly not identified' );
> >> };
> >>
> >> The first subtest passes no problem, the second one though does
> >> not. Each one does pass in isolation, but run together they fail.
> >> It fails because the instance of Test::MockModule never goes out
> >> of scope (the object pointed to by $mocked_user has a refcount of
> >> 2, not of 1) and therefore the mock never gets uninstalled.
> >> CURSES!
> >>
> >> See the issue?
> >>
> >> my $mocked_user = Test::MockModule->new(
> >> 'FreeChat::API::Model::User' );
> >> $mocked_user->mock(
> >> 'search',
> >> sub {
> >> my $self = shift;
> >> my $condition = shift;
> >> my $attributes = shift // {};
> >>
> >> $condition->{'me.id'} = { 'in' => $user_ids };
> >> *return $mocked_user->original( 'search' )->( $self,
> >> $condition, $attributes );*
> >> } );
> >>
> >> It seems that because I called $mocked_user here and have the
> >> entire thing wrapped in a sub, I'm creating a closure causing the
> >> refcount on $mocked_user to be incremented. It seems that one of
> >> the references comes from restrict_search_space's $mocked_user and
> >> the other comes from the subtest's $mocked_user. When I make the
> >> below modification:
> >>
> >> sub restrict_search_space {
> >> my ( $user_ids ) = pos_validated_list( \@_, { isa =>
> >> 'ArrayRef[Int]' } );
> >>
> >> my $mocked_user = Test::MockModule->new(
> >> 'FreeChat::API::Model::User' );
> >> *my $original_method;*
> >> $mocked_user->mock(
> >> 'search',
> >> sub {
> >> my $self = shift;
> >> my $condition = shift;
> >> my $attributes = shift // {};
> >>
> >> $condition->{'me.id'} = { 'in' => $user_ids };
> >> return *$original_method*->( $self, $condition,
> >> $attributes );
> >> } );
> >>
> >> *$original_method = $mocked_user->original( 'search' );*
> >>
> >> return $mocked_user;
> >> }
> >>
> >> Everything worked as expected, at the end of the subtest the
> >> $mocked_user goes out of scope and the method gets unmocked.
> >>
> >> The real question, while this works it seems awfully hacky and
> >> ugly. Is there a better way to do this? Should I have used
> >> weaken somewhere?
> >>
> >> Just thought you all would enjoy commiserating with us. Hope
> >> everyone is having a safe and happy holiday.
> >>
> >> Best Regards,
> >> Robert Stone
> >>
> >> _______________________________________________
> >> Houston mailing list
> >> Houston at pm.org
> >> http://mail.pm.org/mailman/listinfo/houston
> >> Website: http://houston.pm.org/
> >>
> >
> >
> > _______________________________________________
> > Houston mailing list
> > Houston at pm.org
> > http://mail.pm.org/mailman/listinfo/houston
> > Website: http://houston.pm.org/
> >
--
The purpose of software engineering is to control complexity, not to
create it. -- Dr. Pamela
Zave
More information about the Houston
mailing list