Skip Menu |
 

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

Report information
The Basics
Id: 129202
Status: open
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.23
Fixed in: (no value)



Subject: Think about ->cancel
Download (untitled) / with headers
text/plain 662b
Currently cancellation does not work: $ cat cancel-faa.pl #!/usr/bin/perl use strict; use warnings; use Future::IO; use Future::AsyncAwait; my $f = (async sub { await Future::IO->sleep( 1 ); print STDERR "KILL ALL HUMANS\n"; })->(); $f->cancel; Future::IO->sleep( 3 )->get; $ perl cancel-faa.pl KILL ALL HUMANS Whereas by ->then chaining: $ cat cancel-then.pl #!/usr/bin/perl use strict; use warnings; use Future::IO; use Future::AsyncAwait; my $f = Future::IO->sleep( 1 ) ->then( sub { print STDERR "KILL ALL HUMANS\n"; Future->done; }); $f->cancel; Future::IO->sleep( 3 )->get; $ perl cancel-then.pl -- Paul Evans
Possible behaviour: silently stop In this option, the `await` call simply never wakes up again. The entire `async sub` it was contained in just gets dropped. Pad temporaries get reclaimed, etc etc.. and no further code runs. This has a fairly neat consequence most of the time, but doesn't give users any way to provide ->on_cancel blocks as they'd be able were it a straight-up Future. Possible behaviour: await throws an exception In this option, the `await` would throw some sort of exception. There's some kind of precedent in other systems, e.g. Java has its CancellationException. In absence of any try/catch around it, this would propagate up to the toplevel of the `async sub`, which would attempt to ->fail the return future, except since it's already been cancelled nothing happens, so all is fine. This looks fairly similar to the first option, except it does give users a way to provide on_cancel logic if they require it, albeit by trying to match on exception types. Which is never fun. Other ideas welcome... -- Paul Evans
Download (untitled) / with headers
text/plain 412b
I like the exception option better. It doesn't even have to be hard to match: async sub { try { await $some_future; return $some_value; } catch { if (Future::AsyncAwait::WasCancelled($@)) { say "cancelled"; return; } handle_actual_error($@); } }; the exception value can be something like a reference to a single global scalar, so C<WasCancelled> can just use C<eq> to check it.
Download (untitled) / with headers
text/plain 282b
An implementation here, which simply stops execution. It doesn't provide the `async sub` any way to implement ->on_cancel behaviours yet. Further thought needs to be had on a good interface for that, but for now this patch is enough for cases that don't need such. -- Paul Evans
Subject: rt129202.patch
Download rt129202.patch
text/x-diff 2.9k
=== modified file 'lib/Future/AsyncAwait.xs' --- lib/Future/AsyncAwait.xs 2019-04-17 22:59:36 +0000 +++ lib/Future/AsyncAwait.xs 2019-04-18 10:55:35 +0000 @@ -1350,29 +1350,30 @@ return f; } -#define future_is_ready(f) MY_future_is_ready(aTHX_ f) -static int MY_future_is_ready(pTHX_ SV *f) +#define future_is_ready(f) MY_future_check(aTHX_ f, "is_ready") +#define future_is_cancelled(f) MY_future_check(aTHX_ f, "is_cancelled") +static bool MY_future_check(pTHX_ SV *f, const char *method) { dSP; - ENTER_with_name("future_is_ready"); + ENTER_with_name("future_check"); SAVETMPS; PUSHMARK(SP); XPUSHs(f); PUTBACK; - call_method("is_ready", G_SCALAR); + call_method(method, G_SCALAR); SPAGAIN; - int is_ready = POPi; + bool ret = SvTRUEx(POPs); PUTBACK; FREETMPS; - LEAVE_with_name("future_is_ready"); + LEAVE_with_name("future_check"); - return is_ready; + return ret; } #define future_get_to_stack(f, gimme) MY_future_get_to_stack(aTHX_ f, gimme) @@ -1408,6 +1409,23 @@ LEAVE_with_name("future_on_ready"); } +#define future_chain_on_cancel(f1, f2) MY_future_chain_on_cancel(aTHX_ f1, f2) +static void MY_future_chain_on_cancel(pTHX_ SV *f1, SV *f2) +{ + dSP; + + ENTER_with_name("future_chain_on_cancel"); + + PUSHMARK(SP); + XPUSHs(f1); + XPUSHs(f2); + PUTBACK; + + call_method("on_cancel", G_VOID); + + LEAVE_with_name("future_chain_on_cancel"); +} + /* * Custom ops */ @@ -1491,6 +1509,11 @@ TRACEPRINT("ENTER await curcv=%p [%s:%d]\n", curcv, CopFILE(curcop), CopLINE(curcop)); if(state && state->awaiting_future) { + if(future_is_cancelled(state->returning_future)) { + TRACEPRINT(" CANCELLED\n"); + return PL_ppaddr[OP_RETURN](aTHX); + } + I32 orig_height; TRACEPRINT(" RESUME\n"); @@ -1582,6 +1605,8 @@ if(!SvWEAKREF(state->returning_future)) sv_rvweaken(state->returning_future); + future_chain_on_cancel(state->returning_future, state->awaiting_future); + TRACEPRINT("LEAVE await curcv=%p [%s:%d]\n", curcv, CopFILE(curcop), CopLINE(curcop)); return PL_ppaddr[OP_RETURN](aTHX); === added file 't/08await-cancel.t' --- t/08await-cancel.t 1970-01-01 00:00:00 +0000 +++ t/08await-cancel.t 2019-04-18 10:44:16 +0000 @@ -0,0 +1,43 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use Test::More; + +use Future; + +use Future::AsyncAwait; + +my $orig_cxstack_ix = Future::AsyncAwait::__cxstack_ix; + +# ->cancel stops execution +{ + my $called; + + my $f1 = Future->new; + my $f2 = (async sub { + await $f1; + $called++; + })->(); + + $f2->cancel; + $f1->done; + + ok( !$called, 'async sub stops execution after ->cancel' ); +} + +# ->cancel propagates +{ + my $f1 = Future->new; + my $f2 = (async sub { await $f1 })->(); + + $f2->cancel; + + ok( $f1->is_cancelled, 'async sub propagates cancel' ); +} + +is( Future::AsyncAwait::__cxstack_ix, $orig_cxstack_ix, + 'cxstack_ix did not grow during the test' ); + +done_testing;
Download (untitled) / with headers
text/plain 682b
For some really weird reason this causes massive breakages on perls 5.20 and 5.22. 5.24+ is fine, as are 5.16 and 5.18. The failure is that the return future gets DESTROYed far too early leading to lots of broken tests everywhere. I thought at first it was to do with weakened refs, but even if I remove all the sv_rvweaken() calls the problems persist. For now I shall just disable the propagation logic, and associated tests, prior to perl 5.24 and point at this bug. It works on 5.24+ so I'm happy to release in this state even - it's a new feature that didn't exist on any perl version before this, so at worst this is just a new feature but only newer perls. -- 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.