Skip Menu |
 

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

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

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

Bug Information
Severity: (no value)
Broken in: 0.16
Fixed in: 0.17



Subject: Scopestack name upsets under -DDEBUGGING
Download (untitled) / with headers
text/plain 644b
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
Download (untitled) / with headers
text/plain 202b
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
Download rt128164.patch
text/x-diff 1.8k
=== 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);
Download (untitled) / with headers
text/plain 155b
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
Download (untitled) / with headers
text/plain 325b
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
Download rt128164-pt2.patch
text/x-diff 1.8k
=== 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 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.