Bug #88967 for future: Can't call method "cancel" on unblessed reference ...

Wed Sep 25 12:33:36 2013 leonerd-cpan [...] leonerd.org.uk - Ticket created

Subject:

Can't call method "cancel" on unblessed reference ...

Unit tests and so on occasionally throw (in cleanup) Can't call method "cancel" on unblessed reference at /usr/share/perl5/Future.pm line 992 during global destruction. (in cleanup) suggests DESTROY, and the only DESTROY in IO::Async is this one, in Process.pm: 518 sub DESTROY 519 { 520 my $self = shift; 521 $self->{finish_future}->cancel if $self->{finish_future}; 522 } Offhand I can't quite recall if we need or want that in there. If so perhaps it just needs guarding by Devel::GlobalDestruction. May be best to delete it though if it's not actually required. -- Paul Evans

Wed Sep 25 12:43:00 2013 leonerd-cpan [...] leonerd.org.uk - Correspondence added

On Wed Sep 25 12:33:36 2013, PEVANS wrote: Show quoted text

> (in cleanup) Can't call method "cancel" on unblessed reference at > /usr/share/perl5/Future.pm line 992 during global destruction.

Ofcourse, this isn't the "method on undefined value" that I thought it was. This is more subtle. Future.pm line 992 is: 972 sub _new_dependent 973 { 974 shift; # ignore this class 975 my ( $subs ) = @_; 976 977 foreach my $sub ( @$subs ) { 978 blessed $sub and $sub->isa( "Future" ) or Carp::croak "Expected a Future, got $ 979 } 980 981 my $self; 982 $_->is_ready or $self = $_->new, last for @$subs; 983 984 # No non-immediates; just clone the first one anyway then because the 985 # result will necessarily be immediate 986 $self ||= $subs->[0]->new; 987 988 $self->{subs} = $subs; 989 990 $self->on_cancel( sub { 991 foreach my $sub ( @$subs ) { 992 $sub->cancel if !$sub->{ready}; 993 } 994 } ); 995 996 return $self; 997 } So, $sub is an unblessed reference on line 992. But we checked it earlier. The plot curdles... -- Paul Evans

Wed Sep 25 12:45:33 2013 leonerd-cpan [...] leonerd.org.uk - Correspondence added

On Wed Sep 25 12:43:00 2013, PEVANS wrote: Show quoted text

> 990 $self->on_cancel( sub { > 991 foreach my $sub ( @$subs ) { > 992 $sub->cancel if !$sub->{ready}; > 993 } > 994 } );

During global destruction, of course, it's possible that this loop is entered with some subs being undefined refs. So if $sub was undef, the if condition autovivifies it as a HASH reference at which point it's an unblessed reference we can't call cancel on. Oops. A Future bug then. -- Paul Evans

Wed Sep 25 12:45:50 2013 leonerd-cpan [...] leonerd.org.uk - Queue changed from IO-Async to future

Wed Sep 25 12:46:04 2013 leonerd-cpan [...] leonerd.org.uk - Broken in 0.18 added

Wed Sep 25 13:15:08 2013 leonerd-cpan [...] leonerd.org.uk - Correspondence added

On Wed Sep 25 12:45:33 2013, PEVANS wrote: Show quoted text

> During global destruction, of course, it's possible that this loop is > entered with some subs being undefined refs. So if $sub was undef, the > if condition autovivifies it as a HASH reference at which point it's > an unblessed reference we can't call cancel on. Oops. > > A Future bug then.

Attached is a patch to fix it. Will be in 0.19. -- Paul Evans

Subject:

rt88967.patch

=== modified file 'lib/Future.pm' --- lib/Future.pm 2013-09-20 18:11:23 +0000 +++ lib/Future.pm 2013-09-25 17:13:17 +0000 @@ -987,9 +987,11 @@ $self->{subs} = $subs; + # This might be called by a DESTROY during global destruction so it should + # be as defensive as possible (see RT88967) $self->on_cancel( sub { foreach my $sub ( @$subs ) { - $sub->cancel if !$sub->{ready}; + $sub->cancel if $sub and !$sub->{ready}; } } );

Wed Sep 25 13:15:09 2013 leonerd-cpan [...] leonerd.org.uk - Status changed from 'new' to 'patched'

Wed Nov 06 12:11:02 2013 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Was released in 0.19. -- Paul Evans

Wed Nov 06 12:11:02 2013 leonerd-cpan [...] leonerd.org.uk - Status changed from 'patched' to 'resolved'

Wed Nov 06 12:11:02 2013 leonerd-cpan [...] leonerd.org.uk - Fixed in 0.19 added