Skip Menu |
 

This queue is for tickets about the Future-AsyncAwait CPAN distribution.

Report information
The Basics
Id: 130957
Status: resolved
Priority: 0/
Queue: Future-AsyncAwait

People
Owner: Nobody in particular
Requestors: TEAM [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: 0.33
Fixed in: (no value)



Subject: Long-running async subs seem to accumulate on_cancel callbacks
Download (untitled) / with headers
text/plain 260b
Attached an example script - when suspending and resuming, Future instances are accumulated with a suspiciously large number of callbacks queued via ->on_cancel. This looks like an unfortunate side-effect of the cancellation handling from a few versions back?
Subject: await-leak.pl
Download await-leak.pl
text/x-perl 697b
#!/usr/bin/env perl use strict; use warnings; use IO::Async::Loop; use Future::AsyncAwait; use IO::Async::Timer::Periodic; use Devel::MAT::Dumper; my $loop = IO::Async::Loop->new; my @pending; my $count = 0; $loop->add( my $timer = IO::Async::Timer::Periodic->new( interval => 0.0001, on_tick => sub { if($count > 1000) { Devel::MAT::Dumper::dump('out.pmat'); exit 0; } my @f = splice @pending; $_->done for @f; } ) ); $timer->start; (async sub { while(1) { ++$count; my $f = $loop->new_future; push @pending, $f; await $f; } })->()->get;
Download (untitled) / with headers
text/plain 125b
Turns out that this is quite easy to reproduce without IO::Async loaded - simpler example and potential test script attached.
Subject: await-leak-plain.pl
Download await-leak-plain.pl
text/x-perl 423b
#!/usr/bin/env perl use strict; use warnings; use Future::AsyncAwait; use Devel::MAT::Dumper; my @pending; my $count = 0; (async sub { while(1) { ++$count; my $f = Future->new; push @pending, $f; await $f; } })->()->retain; while(1) { my @f = splice @pending; $_->done for @f; if($count > 1000) { Devel::MAT::Dumper::dump('out.pmat'); exit 0; } }
Subject: await-leak.t
Download await-leak.t
text/x-perl 472b
#!/usr/bin/env perl use strict; use warnings; use Future::AsyncAwait; use Test::More; use Test::Refcount; use Scalar::Util qw(weaken); my @pending; my $count = 0; (async sub { while(1) { ++$count; my $f = Future->new; push @pending, $f; await $f; } })->()->retain; for(1..10) { my @f = splice @pending; for(@f) { $_->done; is_oneref($_); weaken($_); is($_, undef); } } done_testing;
Download (untitled) / with headers
text/plain 993b
Looks like the issue is that `$x->on_cancel($y)` is keeping a strong ref on `$y` - even when `$y` is marked as ready, it won't be a candidate for destruction until `$x` is marked as ready. Updating `on_cancel` in Future.pm to clear out referenced futures when they are marked as ready causes the test case to pass: sub on_cancel { my $self = shift; my ( $code ) = @_; my $is_future = blessed( $code ) && $code->isa( "Future" ); $is_future or _callable( $code ) or Carp::croak "Expected \$code to be callable or a Future in ->on_cancel"; $self->{ready} and return $self; if($is_future) { use Scalar::Util qw(weaken refaddr); use List::UtilsBy qw(extract_by); weaken(my $weak_self = $self); $code->on_ready(sub { my $self = $weak_self or return; my $f = shift; extract_by { refaddr($_) == refaddr($f) } @{$self->{on_cancel}}; }); } push @{ $self->{on_cancel} }, $code; return $self; }
Download (untitled) / with headers
text/plain 872b
On Sun Nov 10 13:01:39 2019, TEAM wrote: Show quoted text
> Looks like the issue is that `$x->on_cancel($y)` is keeping a strong > ref on `$y` - even when `$y` is marked as ready, it won't be a > candidate for destruction until `$x` is marked as ready.
That sounds a good description of what's going on. Show quoted text
> Updating `on_cancel` in Future.pm to clear out referenced futures when > they are marked as ready causes the test case to pass:
I found a somewhat better(?) way to basically do that. Each Future now stores a (weakened) list of SCALAR refs to other places where it is referenced; scalars it should undef when it is marked as ready. The `on_cancel` method then stores a ref in the target $code future's list. This ensures that two pending futures form a strong ref in the cancellation chain, but once the latter one becomes resolved the former one can let go of it. -- Paul Evans
Subject: rt130957.patch
Download rt130957.patch
text/x-diff 1.4k
=== modified file 'lib/Future.pm' --- lib/Future.pm 2019-06-13 13:41:38 +0000 +++ lib/Future.pm 2019-11-11 14:27:22 +0000 @@ -377,6 +377,8 @@ } delete $self->{on_cancel}; + $_ and undef $$_ for @{ $self->{undef_on_ready} }; + my $callbacks = delete $self->{callbacks} or return; my $cancelled = $self->{cancelled}; @@ -716,6 +718,10 @@ $self->{ready} and return $self; push @{ $self->{on_cancel} }, $code; + if( $is_future ) { + push @{ $code->{undef_on_ready} }, \$self->{on_cancel}[-1]; + weaken $code->{undef_on_ready}[-1]; + } return $self; } @@ -1037,6 +1043,7 @@ $self->{cancelled}++; foreach my $code ( reverse @{ $self->{on_cancel} || [] } ) { + defined $code or next; my $is_future = blessed( $code ) && $code->isa( "Future" ); $is_future ? $code->cancel : $code->( $self ); === modified file 't/02cancel.t' --- t/02cancel.t 2017-11-28 17:32:24 +0000 +++ t/02cancel.t 2019-11-11 14:27:22 +0000 @@ -84,8 +84,18 @@ { my $f1 = Future->new; my $f2 = Future->new; + my $f3 = Future->new; $f1->on_cancel( $f2 ); + $f1->on_cancel( $f3 ); + + is_oneref( $f1, '$f1 has refcount 1 after on_cancel chaining' ); + is_refcount( $f2, 2, '$f2 has refcount 2 after on_cancel chaining' ); + is_refcount( $f3, 2, '$f3 has refcount 2 after on_cancel chaining' ); + + $f3->done; + is_oneref( $f3, '$f3 has refcount 1 after done in cancel chain' ); + my $cancelled; $f2->on_cancel( sub { $cancelled++ } );
Future 0.42 fixes this one. -- Paul Evans


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.