Weird. SvREFCNT(PL_errgv) increases by 1 the first time one of the tests is run, but is stable afterwards. Unsure quite what this is but I can imagine it being some sort of lazy creation of something somewhere. Easy fix is to run the test twice but only check refcount on the second run. This seems to work.
Overall then I think this is now fixed.
--
Paul Evans
=== modified file 'Build.PL'
--- Build.PL 2019-01-12 20:06:45 +0000
+++ Build.PL 2019-06-25 19:35:33 +0000
@@ -27,6 +27,7 @@
},
test_requires => {
'Test::More' => '0.88', # done_testing
+ 'Test::Refcount' => '0.09',
},
configure_requires => {
'Module::Build' => '0.4004', # test_requires
=== modified file 'lib/Future/AsyncAwait.xs'
--- lib/Future/AsyncAwait.xs 2019-06-17 15:58:33 +0000
+++ lib/Future/AsyncAwait.xs 2019-06-25 22:35:37 +0000
@@ -279,16 +279,54 @@
SuspendedFrame *frame;
for(frame = state->frames; frame; frame = frame->next) {
- if(frame->stacklen) {
- int i;
- for(i = 0; i < frame->stacklen; i++)
- ret += DMD_ANNOTATE_SV(sv, frame->stack[i], "a suspended stack temporary");
- }
-
- if(frame->mortallen) {
- int i;
- for(i = 0; i < frame->mortallen; i++)
- ret += DMD_ANNOTATE_SV(sv, frame->mortals[i], "a suspended mortal");
+ int i;
+
+ for(i = 0; i < frame->stacklen; i++)
+ ret += DMD_ANNOTATE_SV(sv, frame->stack[i], "a suspended stack temporary");
+
+ for(i = 0; i < frame->mortallen; i++)
+ ret += DMD_ANNOTATE_SV(sv, frame->mortals[i], "a suspended mortal");
+
+ for(i = 0; i < frame->savedlen; i++) {
+ struct Saved *saved = &frame->saved[i];
+ switch(saved->type) {
+#ifdef SAVEt_CLEARPADRANGE
+ case SAVEt_CLEARPADRANGE:
+#endif
+ case SAVEt_CLEARSV:
+ case SAVEt_INT_SMALL:
+ case SAVEt_DESTRUCTOR_X:
+#ifdef SAVEt_STRLEN
+ case SAVEt_STRLEN:
+#endif
+ case SAVEt_SET_SVFLAGS:
+ /* Nothing interesting */
+ break;
+
+ case SAVEt_FREEPV:
+ /* This is interesting but a plain char* pointer so there's nothing
+ * we can do with it in Devel::MAT */
+ break;
+
+ case SAVEt_COMPPAD:
+ ret += DMD_ANNOTATE_SV(sv, saved->cur.ptr, "a suspended SAVEt_COMPPAD");
+ break;
+
+ case SAVEt_FREESV:
+ ret += DMD_ANNOTATE_SV(sv, saved->saved.sv, "a suspended SAVEt_FREESV");
+ break;
+
+ case SAVEt_SV:
+ ret += DMD_ANNOTATE_SV(sv, (SV *)saved->u.gv, "a suspended SAVEt_SV target GV");
+ ret += DMD_ANNOTATE_SV(sv, saved->cur.sv, "a suspended SAVEt_SV current value");
+ ret += DMD_ANNOTATE_SV(sv, saved->saved.sv, "a suspended SAVEt_SV saved value");
+ break;
+
+ case SAVEt_PADSV_AND_MORTALIZE:
+ ret += DMD_ANNOTATE_SV(sv, saved->cur.sv, "a suspended SAVEt_PADSV_AND_MORTALIZE current value");
+ ret += DMD_ANNOTATE_SV(sv, saved->saved.sv, "a suspended SAVEt_PADSV_AND_MORTALIZE saved value");
+ break;
+ }
}
}
@@ -383,6 +421,12 @@
Safefree(saved->cur.ptr);
break;
+ case SAVEt_SV:
+ SvREFCNT_dec(saved->u.gv);
+ SvREFCNT_dec(saved->saved.sv);
+ SvREFCNT_dec(saved->cur.sv);
+ break;
+
case SAVEt_PADSV_AND_MORTALIZE:
SvREFCNT_dec(saved->cur.sv);
break;
@@ -635,7 +679,7 @@
saved->saved.sv = val; /* steal ownership */
/* restore it for now */
- GvSV(gv) = SvREFCNT_inc(val);
+ GvSV(gv) = val;
break;
}
@@ -1226,7 +1270,7 @@
break;
case SAVEt_SV:
- save_pushptrptr(saved->u.gv, saved->saved.sv, SAVEt_SV);
+ save_pushptrptr(saved->u.gv, SvREFCNT_inc(saved->saved.sv), SAVEt_SV);
SvREFCNT_dec(GvSV(saved->u.gv));
GvSV(saved->u.gv) = saved->cur.sv;
=== added file 't/15local-errsv.t'
--- t/15local-errsv.t 1970-01-01 00:00:00 +0000
+++ t/15local-errsv.t 2019-06-25 22:35:06 +0000
@@ -0,0 +1,67 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Test::More;
+use Test::Refcount 0.09 import => [qw( is_refcount refcount )];
+
+use Future;
+
+use Future::AsyncAwait;
+
+my $orig_cxstack_ix = Future::AsyncAwait::__cxstack_ix;
+
+my $errgv_ref = \*@;
+
+# $@ can be localized
+# For some odd reason, SvREFCNT(PL_errgv) increases by 1 the first time
+# this code is run, but is stable thereafter. So only test it the second
+# time. Perhaps something somewhere needs to get lazily created?
+foreach my $idx ( 1, 2 )
+{
+ my $errsv_refcount = refcount(\$@);
+ my $errgv_refcount = refcount($errgv_ref);
+
+ my $f1 = Future->new;
+ my $fret = (async sub {
+ local $@ = "inside";
+ await $f1;
+ return $@;
+ })->();
+
+ $@ = "OUTSIDE";
+ $f1->done;
+ is( scalar $fret->get, "inside", 'result from async sub with local $@' );
+
+ is_refcount( \$@, $errsv_refcount, '$@ refcount preserved' );
+ is_refcount( $errgv_ref, $errgv_refcount, '*@ refcount preserved' ) if $idx > 1;
+}
+
+# localised $@ plays nicely with eval{}
+{
+ my $errsv_refcount = refcount(\$@);
+ my $errgv_refcount = refcount($errgv_ref);
+
+ my $f1 = Future->new;
+ my $fret = (async sub {
+ local $@ = "inside";
+ await $f1;
+ eval {
+ die "oopsie\n";
+ };
+ return $@;
+ })->();
+
+ $@ = "OUTSIDE";
+ $f1->done;
+ is( scalar $fret->get, "oopsie\n", 'result from eval { die }' );
+
+ is_refcount( \$@, $errsv_refcount, '$@ refcount preserved' );
+ is_refcount( $errgv_ref, $errgv_refcount, '*@ refcount preserved' );
+}
+
+is( Future::AsyncAwait::__cxstack_ix, $orig_cxstack_ix,
+ 'cxstack_ix did not grow during the test' );
+
+done_testing;
=== modified file 't/42unresolved.t'
--- t/42unresolved.t 2019-04-26 12:55:27 +0000
+++ t/42unresolved.t 2019-06-25 22:05:51 +0000
@@ -4,6 +4,7 @@
use warnings;
use Test::More;
+use Test::Refcount 0.09 import => [qw( is_refcount refcount )];
use Future;
@@ -12,6 +13,8 @@
my $orig_cxstack_ix = Future::AsyncAwait::__cxstack_ix;
my $file = quotemeta __FILE__;
+my $errgv_ref = \*@;
+
async sub identity
{
return await $_[0];
@@ -78,6 +81,22 @@
pass( "abandoned foreach loop does not crash" );
}
+# abandoned local $@
+{
+ my $errsv_refcount = refcount(\$@);
+ my $errgv_refcount = refcount($errgv_ref);
+
+ my $f1 = Future->new;
+ my $fret = (async sub { local $@; await $f1 })->();
+
+ undef $fret;
+ undef $f1;
+ pass( "abandoned local \$@ does not crash" );
+
+ is_refcount( \$@, $errsv_refcount, '$@ refcount preserved' );
+ is_refcount( $errgv_ref, $errgv_refcount, '*@ refcount preserved' );
+}
+
is( Future::AsyncAwait::__cxstack_ix, $orig_cxstack_ix,
'cxstack_ix did not grow during the test' );
=== modified file 't/81memory-growth.t'
--- t/81memory-growth.t 2019-02-02 17:19:16 +0000
+++ t/81memory-growth.t 2019-06-24 15:12:20 +0000
@@ -32,4 +32,16 @@
calls => 10000,
'async/await does not grow memory';
+sub abandoned
+{
+ my $f1 = Future->new;
+ my $fret = (async sub { local $@; await $f1 })->();
+ undef $fret;
+ undef $f1;
+}
+
+no_growth \&abandoned,
+ calls => 10000,
+ 'abandoned async sub does not grow memory';
+
done_testing;
=== added file 't/82devel-mat-dumper-helper.t'
--- t/82devel-mat-dumper-helper.t 1970-01-01 00:00:00 +0000
+++ t/82devel-mat-dumper-helper.t 2019-06-24 15:10:14 +0000
@@ -0,0 +1,30 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Test::More;
+BEGIN {
+ eval { require Devel::MAT; } or
+ plan skip_all => "No Devel::MAT";
+
+ require Devel::MAT::Dumper;
+}
+
+use Future;
+
+use Future::AsyncAwait;
+
+my $f1 = Future->new;
+my $fret = (async sub { local $@; await $f1 })->();
+
+( my $file = __FILE__ ) =~ s/\.t$/.pmat/;
+Devel::MAT::Dumper::dump( $file );
+END { unlink $file if -f $file }
+
+$f1->done;
+$fret->get;
+
+pass( "did not crash" );
+
+done_testing;
