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

Report information
The Basics
Id:
128164
Status:
resolved
Priority:
Low/Low

People
Owner:
Nobody in particular
Requestors:
leonerd-cpan [...] leonerd.org.uk
Cc:
AdminCc:

BugTracker
Severity:
(no value)
Broken in:
0.16
Fixed in:
0.17



Subject: Scopestack name upsets under -DDEBUGGING
On -DDEBUGGING perl builds, one test reliably fails: http://www.cpantesters.org/cpan/report/51b44220-0fde-11e9-92ed-2d3e4b661d59 perl: lib/Future/AsyncAwait.xs:1005: MY_future_done_from_stack: Assertion `((char*)PL_scopestack_name[PL_scopestack_ix-1] == (char*)"future_done_from_stack") || strEQ(PL_scopestack_name[PL_scopestack_ix-1], "future_done_from_stack")' failed. t/06await-nested.t ..... No subtests run On comparable platforms that don't have -DDEBUGGING this test runs OK, which suggests it is simply a failure to maintain the scopestack name entry correctly, rather than any underlying stack discipline failure. -- Paul Evans
Possibly fixed here, though it looks a little fragile - with ENTER only being invoked on -DDEBUGGING builds. That or not doesn't appear to break anything though, so.... kinda confusing. -- Paul Evans
Subject: rt128164.patch
=== modified file 'lib/Future/AsyncAwait.xs' --- lib/Future/AsyncAwait.xs 2019-01-04 16:26:54 +0000 +++ lib/Future/AsyncAwait.xs 2019-01-04 18:10:19 +0000 @@ -640,14 +640,21 @@ suspend_block(frame, cx); -#ifdef DEBUGGING - frame->scopename = PL_scopestack_name[PL_scopestack_ix-1]; -#endif - /* We'll mutate PL_scopestack_ix but it doesn't matter as dounwind() will - * put it right at the end. Do this unconditionally to avoid divergent - * behaviour between -DDEBUGGING builds and non. - */ - --PL_scopestack_ix; + if(cx->blk_oldscopesp <= PL_scopestack_ix) { +#ifdef DEBUGGING + frame->scopename = PL_scopestack_name[PL_scopestack_ix-1]; +#endif + /* We'll mutate PL_scopestack_ix but it doesn't matter as dounwind() will + * put it right at the end. Do this unconditionally to avoid divergent + * behaviour between -DDEBUGGING builds and non. + */ + --PL_scopestack_ix; + } + else { +#ifdef DEBUGGING + frame->scopename = NULL; +#endif + } /* ref: * https://perl5.git.perl.org/perl.git/blob/HEAD:/cop.h @@ -906,6 +913,17 @@ PERL_CONTEXT *cx; +#if defined(DEBUGGING) && HAVE_PERL_VERSION(5, 24, 0) + /* Apparently, we have to ENTER here on versions of perl before the Grand + * Context Stack Refactoring at 5.24, or else the scopename logic will get + * upset. See + * https://rt.cpan.org/Ticket/Display.html?id=128164 + */ + if(frame->scopename) { + ENTER; + } +#endif + switch(frame->type) { case CXt_BLOCK: cx = cx_pushblock(CXt_BLOCK, frame->gimme, PL_stack_sp, PL_savestack_ix); @@ -964,7 +982,9 @@ resume_block(frame, cx); #ifdef DEBUGGING - PL_scopestack_name[PL_scopestack_ix-1] = frame->scopename; + if(frame->scopename) { + PL_scopestack_name[PL_scopestack_ix-1] = frame->scopename; + } #endif Safefree(frame);
I'm going to mark resolved for now, but it may turn out that the oddness of the fix isn't quite resolved yet and we might come back to it. -- Paul Evans
I knew this didn't feel quite right somehow. Turns out the hax/cx_pushblock I copied from mauke was wrong. It shouldn't ENTER; SAVETMPS. Instead, each callsite should do what is the right thing, but only on pre-5.24 perls. Attached patch. This also means I can delete the frame->scopename hacks as well. -- Paul Evans
Subject: rt128164-pt2.patch
=== modified file 'hax/cx_pushblock.c.inc' --- hax/cx_pushblock.c.inc 2018-01-15 18:20:52 +0000 +++ hax/cx_pushblock.c.inc 2019-04-13 19:35:37 +0000 @@ -9,9 +9,6 @@ PERL_CONTEXT *cx; assert(saveix == PL_savestack_ix); - ENTER; - SAVETMPS; - PUSHBLOCK(cx, t, sp); return cx; } === modified file 'lib/Future/AsyncAwait.xs' --- lib/Future/AsyncAwait.xs 2019-04-13 19:19:46 +0000 +++ lib/Future/AsyncAwait.xs 2019-04-13 19:35:37 +0000 @@ -1156,11 +1156,20 @@ switch(frame->type) { case CXt_BLOCK: +#if !HAVE_PERL_VERSION(5, 24, 0) + ENTER_with_name("block"); + SAVETMPS; +#endif cx = cx_pushblock(CXt_BLOCK, frame->gimme, PL_stack_sp, PL_savestack_ix); /* nothing else special */ break; case CXt_LOOP_PLAIN: +#if !HAVE_PERL_VERSION(5, 24, 0) + ENTER_with_name("loop1"); + SAVETMPS; + ENTER_with_name("loop2"); +#endif cx = cx_pushblock(frame->type, frame->gimme, PL_stack_sp, PL_savestack_ix); /* don't call cx_pushloop_plain() because it will get this wrong */ cx->blk_loop = frame->el.loop; @@ -1174,6 +1183,11 @@ #endif case CXt_LOOP_LAZYSV: case CXt_LOOP_LAZYIV: +#if !HAVE_PERL_VERSION(5, 24, 0) + ENTER_with_name("loop1"); + SAVETMPS; + ENTER_with_name("loop2"); +#endif cx = cx_pushblock(frame->type, frame->gimme, PL_stack_sp, PL_savestack_ix); /* don't call cx_pushloop_plain() because it will get this wrong */ cx->blk_loop = frame->el.loop; @@ -1199,6 +1213,10 @@ if(CATCH_GET) panic("Too late to docatch()\n"); +#if !HAVE_PERL_VERSION(5, 24, 0) + ENTER_with_name("eval_scope"); + SAVETMPS; +#endif cx = cx_pushblock(CXt_EVAL|CXp_TRYBLOCK, frame->gimme, PL_stack_sp, PL_savestack_ix); cx_pusheval(cx, frame->el.eval.retop, NULL);


This service runs on Request Tracker, is sponsored by The Perl Foundation, and maintained by Best Practical Solutions.

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