Cancellation may not always immediately happen at checkpoints

Updated issue text (after thinking about it a bit)

Summary

Any awaitable function in Trio (i.e., await trio.foo(...)) is guaranteed to be a checkpoint. That means, aside from being a scheduling point (which is not relevant to this issue), that it is a cancellation point: i.e., if the function is called from within a cancellation scope that is cancelled (subject to shielding rules) then it will raise trio.Cancelled. Of course, a function that is waiting (in a sleep, for I/O data, etc.) can also be interrupted by a cancellation (with a few well-documented exceptions).

But what if the cancellation scope becomes cancelled just as the function is finishing? Then, surprisingly (to me), it might not raise trio.Cancelled. Here's a simple example:

import trio

async def wait_for_event(ev: trio.Event):
    await ev.wait()
    print("If this prints then wait() did not raise")

async def main():
    async with trio.open_nursery() as n:
        ev = trio.Event()
        n.start_soon(wait_for_event, ev)
        await trio.sleep(0.1)
        ev.set()
        n.cancel_scope.cancel()

trio.run(main)

This will print the message. That's because await ev.wait() simply returns, despite being in a cancelled scope at that point.

What's more, if you swap the last two lines of main() (ev.set() and n.cancel_scope.cancel()) then the message will not be printed, because await ev.wait() raises a trio.Cancelled exception instead. I would expect that the behaviour depends on the overall state at that moment (the event is in a set state and in a cancelled context, so one or the other deterministically wins). Instead, it depends on the order things happened, even between checkpoints. I can see why that is likely to have happened internally, but it still seems against the spirit of Trio.

Why I care

Here's an almost-concrete example of why this is a problem, using my little aioresult library:

async def use_result(rc: aioresult.ResultCapture):
    # ...
    await rc.wait_done()
    val = rc.result()
    # ... use val ...

async def main():
    async with trio.open_nursery() as n:
        rc = aioresult.ResultCapture(n, produce_result)
        n.start_soon(use_result, rc)
        # ... some time later ...
        if some_condition:
            n.cancel_scope.cancel()

Here's the risk: produce_result() and use_result() are both cancelled but, await rc.wait_done() returns without exception (because when produce_result() finished with trio.Cancelled that set the event inside aioresult.ResultCapture). That means that rc.result() raises an aioresult.TaskFailedException (because the routine it wraps raised an exception). Which means that the simple cancellation get converted into a full-fat exception propagated out of the nursery!

Now, I don't think this can happen because the event gets cancelled just before it's set in this case. But it seems like it's got a fragile dependency on undocumented behaviour here.

The request

I think there should be changes relating to this:

• Trio's docs about checkpoints (the two main places listed in "documentation" in the original issue text) make it really clear that cancellation is not guaranteed to be checked as the call is finishing. In other words, if surrounding cancel scope is cancelled while await trio.foo() is in progress then it is possible for the routine to return without raising trio.Cancelled.
• BUT, for some particular functions where it's possible, the functions are changed to add that guarantee and that is documented. That would include, IMO, at least Event.wait() and await trio.MemoryReceiveChannel.receive(), and probably also trio.lowlevel.ParkingLot. I'm not so interested in the lower-level synchronization primitives (CapacityLimiter, Lock, Semaphore and Condition) but I suppose it applies to those too.

I've not put my money where my mouth is by submitting a pull request!

Implementation

One problem with this idea is that I'm not sure how it could be efficiently implemented. Ideally, late in the body of these functions (after waiting for their condition to be true) we would re-check for cancellation but without doing a schedule point because we have just been scheduled. That means that we want a sync-coloured cancellation check, but that was suggested 5 years ago in #961 and is not done yet. - I now think this would be best implemented by optionally allowing trio.lowlevel.wait_task_reschedule() to still call the abort function, and therefore end up raising trio.Cancelled, even if trio.lowlevel.reschedule() has been called. I plan to add a comment with further details but that's the key idea. In the case of trio.Event, aside from passing a parameter to enable that behaviour, no other code changes whatsoever would be needed to change ("fix") its behaviour. For other synchronisation primitives (e.g., memory channels), it's a different matter. (To be continued...)

Original issue text

Start with the checkpoint and cancel-point sections in reference-core and design.md, then inspect lowlevel.wait_task_rescheduled and the documented Event.wait and MemoryReceiveChannel.receive entry points. The work is complete when the cancellation guarantees are decided, the affected behavior is implemented where appropriate, and the documentation clearly describes the result.