Bug #122920 for future: ->without_cancel doesn't hold strong ref to its creator

Tue Aug 29 07:13:32 2017 leonerd-cpan [...] leonerd.org.uk - Ticket created

Subject:

->without_cancel doesn't hold strong ref to its creator

my $f2 = $f1->without_cancel; $f2 does not strongly hold $f1. So if that was the only thing that would hold it, then it is liable to disappear. Future->wait_any( somefunc()->without_cancel, timeout_future( ... ), ) -- Paul Evans

Mon Nov 27 12:11:13 2017 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Fixed -- Paul Evans

Subject:

rt122920.patch

=== modified file 'lib/Future.pm' --- lib/Future.pm 2017-11-27 17:01:28 +0000 +++ lib/Future.pm 2017-11-27 17:10:31 +0000 @@ -1487,6 +1487,8 @@ } }); + $new->{orig} = $self; # just to strongref it - RT122920 + return $new; } === modified file 't/02cancel.t' --- t/02cancel.t 2015-07-28 15:12:47 +0000 +++ t/02cancel.t 2017-11-27 17:10:31 +0000 @@ -6,6 +6,7 @@ use Test::More; use Test::Fatal; use Test::Identity; +use Test::Refcount; use Future; @@ -102,7 +103,10 @@ # without_cancel { my $f1 = Future->new; + is_oneref( $f1, '$f1 has single reference initially' ); + my $f2 = $f1->without_cancel; + is_refcount( $f1, 2, '$f1 has two references after ->without_cancel' ); $f2->cancel; ok( !$f1->is_cancelled, '$f1 not cancelled just because $f2 is' );

Mon Nov 27 12:11:14 2017 leonerd-cpan [...] leonerd.org.uk - Status changed from 'new' to 'patched'

Tue Nov 28 07:15:55 2017 TEAM [...] cpan.org - Correspondence added

Is it worth deleting $self->{orig} in ->on_ready? Might be a bit contrived, but it seems possible to end up with a dep loop when things are chained on both the original and the ->without_cancel Future.

Tue Nov 28 09:55:00 2017 leonerd-cpan [...] leonerd.org.uk - Correspondence added

On Tue Nov 28 07:15:55 2017, TEAM wrote: Show quoted text

> Is it worth deleting $self->{orig} in ->on_ready? Might be a bit > contrived, but it seems possible to end up with a dep loop when things > are chained on both the original and the ->without_cancel Future.

Oops, verymuch so. Once completed, a Future isn't supposed to hold on to its past history at all, for exactly that sort of cycle-avoiding problem. I'll add that, thanks -- Paul Evans

Tue Nov 28 10:07:32 2017 leonerd-cpan [...] leonerd.org.uk - Correspondence added

On Tue Nov 28 09:55:00 2017, PEVANS wrote: Show quoted text

> Oops, verymuch so. Once completed, a Future isn't supposed to hold on > to its past history at all, for exactly that sort of cycle-avoiding > problem. I'll add that, thanks

Fixed. Hopefully. -- Paul Evans

Subject:

rt122920-pt2.patch

=== modified file 'lib/Future.pm' --- lib/Future.pm 2017-11-27 22:11:08 +0000 +++ lib/Future.pm 2017-11-28 15:06:58 +0000 @@ -1523,6 +1523,7 @@ }); $new->{orig} = $self; # just to strongref it - RT122920 + $new->on_ready( sub { undef $_[0]->{orig} } ); return $new; } === modified file 't/02cancel.t' --- t/02cancel.t 2017-11-27 21:00:45 +0000 +++ t/02cancel.t 2017-11-28 15:06:58 +0000 @@ -117,6 +117,7 @@ ok( $f3->is_ready, '$f3 ready when $f1 is' ); is_deeply( [ $f3->get ], [ "result" ], 'result of $f3' ); + is_oneref( $f1, '$f1 has one reference after done' ); } done_testing;

Wed Nov 29 14:12:06 2017 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Fixed in 0.37 -- Paul Evans

Wed Nov 29 14:12:06 2017 leonerd-cpan [...] leonerd.org.uk - Status changed from 'patched' to 'resolved'

Wed Nov 29 14:12:06 2017 leonerd-cpan [...] leonerd.org.uk - Fixed in 0.37 added