Show quoted text> At suspend time we'll have to measure them relative to some point we
> can ensure we put back in place later on. At that point of execution,
> I *think* PL_stack_sp should be stable enough that we can measure them
> backwards, down from the top.
Attached patch does this.
Tests now pass on 5.24+, but still fail on the previous versions of perl - which don't use CXt_LOOP_LIST. I suspect they'll have some other, analogous bug in their handling.
--
Paul Evans
=== modified file 'lib/Future/AsyncAwait.xs'
--- lib/Future/AsyncAwait.xs 2019-04-18 16:16:19 +0000
+++ lib/Future/AsyncAwait.xs 2019-04-18 16:22:03 +0000
@@ -126,6 +126,10 @@
} eval;
struct block_loop loop;
} el;
+
+ /* for debugging purposes */
+ SV *loop_list_first_item;
+
#ifdef HAVE_ITERVAR
SV *itervar;
#endif
@@ -749,6 +753,25 @@
*/
if(frame->stacklen || frame->marklen)
panic("ARGH CXt_LOOP_LIST frame tried to steal stack values or marks");
+
+ /* The various fields in the context structure store absolute stack
+ * heights as offsets from PL_stack_base directly. When we get
+ * resumed the stack will probably not be the same absolute height
+ * at this point, so we'll have to store them relative to something
+ * fixed.
+ * We'll adjust them to be upside-down, counting -backwards- from
+ * the current stack height.
+ */
+ I32 height = PL_stack_sp - PL_stack_base;
+
+ if(cx->blk_oldsp != height)
+ panic("ARGH suspending CXt_LOOP_LIST frame with blk_oldsp != stack height");
+
+ /* First item is at [1] oddly, not [0] */
+ frame->loop_list_first_item = PL_stack_base[cx->blk_loop.state_u.stack.basesp+1];
+
+ frame->el.loop.state_u.stack.basesp = height - frame->el.loop.state_u.stack.basesp;
+ frame->el.loop.state_u.stack.ix = height - frame->el.loop.state_u.stack.ix;
break;
#endif
}
@@ -1225,6 +1248,33 @@
Safefree(frame->mortals);
frame->mortals = NULL;
}
+
+ switch(frame->type) {
+#if HAVE_PERL_VERSION(5, 24, 0)
+ case CXt_LOOP_LIST: {
+ I32 height = PL_stack_sp - PL_stack_base;
+
+ cx->blk_loop.state_u.stack.basesp = height - cx->blk_loop.state_u.stack.basesp;
+ cx->blk_loop.state_u.stack.ix = height - cx->blk_loop.state_u.stack.ix;
+
+ /* For consistency; check that the first SV in the list is in the right
+ * place. If so we presume the others are
+ */
+ if(PL_stack_base[cx->blk_loop.state_u.stack.basesp+1] == frame->loop_list_first_item)
+ break;
+
+ /* First item is at [1] oddly, not [0] */
+ fprintf(stderr, "F:AA: consistency check resume LOOP_LIST with first=%p:",
+ frame->loop_list_first_item);
+ debug_sv_summary(frame->loop_list_first_item);
+ fprintf(stderr, " stackitem=%p:", PL_stack_base[frame->el.loop.state_u.stack.basesp + 1]);
+ debug_sv_summary(PL_stack_base[frame->el.loop.state_u.stack.basesp]);
+ fprintf(stderr, "\n");
+ panic("ARGH CXt_LOOP_LIST consistency check failed");
+ break;
+ }
+#endif
+ }
}
#define suspendedstate_resume(state, cv) MY_suspendedstate_resume(aTHX_ state, cv)
=== modified file 't/22context-foreach.t'
--- t/22context-foreach.t 2018-01-20 14:09:54 +0000
+++ t/22context-foreach.t 2019-04-18 16:22:03 +0000
@@ -119,6 +119,24 @@
is( scalar $fret->get, "awaited twice", '$fret now ready after foreach with two awaits' );
}
+# RT#129215
+{
+ my @F = map { Future->new } 0, 1, 2;
+ my $fret = (async sub {
+ foreach my $idx ( 0, 1, 2 ) {
+ await $F[$idx];
+ }
+ return "OK";
+ })->();
+
+ # Arrange for the stack to be at different heights on each resume
+ my $tmp = do { 1 + ( $F[0]->done // 0 ) };
+ $tmp = [ 2, 3, [ $F[1]->done ] ];
+ $tmp = 4 * ( 6 + ( $F[2]->done // 0 ) );
+
+ is( scalar $fret->get, "OK", '$fret now ready after differing stack resumes' );
+}
+
# TODO:
# This ought to be a compiletime check. That's hard right now so for now
# it's a runtime check
