Cancelling coroutines: can't pend throw to just-started generator.

My code heavily relies on coroutines which get canceled quite often.
Occasionally it happens, that a coroutine gets put on the loop but then canceled before it gets executed for the first time.
Apparently this is not possible as it throws a TypeError:

import uasyncio as asyncio
>>> async def test():
...     print("test")
>>> t=test()
>>> async def cancel():
...     asyncio.cancel(t)
>>> loop=asyncio.get_event_loop()
>>> loop.create_task(cancel())
>>> loop.create_task(t)
>>> loop.run_forever()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "uasyncio/core.py", line 109, in run_forever
  File "<stdin>", line 2, in cancel
  File "uasyncio/core.py", line 273, in cancel
TypeError: can't pend throw to just-started generator

This is incompatible to CPython and causes some problems for me because suddenly there is no way to cancel this coroutine unless I wait until it yields for the first time.
An example of where this might happen is if an asyncio task is created when an MQTT connection is established and the same task gets cancelled when the connection breaks. So if the connection breaks until the task gets executed, it would run into this error.

Is this fixable or just the way generators work in micropython?

Is this fixable or just the way generators work in micropython?

MicroPython uses pend_throw() on generators (an extension to CPython) to raise an exception into a generator the next time it is iterated/yielded upon. This will only work for generators that have started.

Note that a generator can catch the injected CancelledError and ignore it, ie not get cancelled, or delay the cancellation. That's only possible because the generator has started execution.

The way cancellation of tasks is exposed to the user in uasyncio is different to how CPython does it, and the behaviour seen above is a consequence of this difference.

Do you know in CPython if it's possible to do this, cancel a task before it has started? Does it inject an exception or just never even start the generator/task?

Since a generator can only be cancelled after it has first yielded, it could have executed a lot of synchronous code before the cancellation even gets to it. So as half-baked workaround every coroutine would need to yield right at the start to ensure it can catch a Cancellation right at the start while the caller (the coroutine that wants to cancel the generator) has to try to cancel it until it doesn't throw an error. So a workaround in micropython would look like this:

async def foo():
    await asyncio.sleep(0)  # yield once to make it cancellable
    try:
        print("foo")
        await asyncio.sleep(1)
    except asyncio.CancelledError:
        print("foo got canceled")


async def main():
    t = foo()
    loop.create_task(t)
    while True:
        try:
            asyncio.cancel(t)
            break
        except TypeError:  # can't pend throw to just-started generator
            await asyncio.sleep(0)
    await asyncio.sleep(1)
    print("main done")


loop.run_until_complete(main())

I know CPython does it differently with Tasks but they can be canceled before they even yield and then never get executed at all (so it also doesn't inject the CancelledError, the Task just doesn't get executed)

import asyncio

async def foo():
    try:
        print("Foo started")
        await asyncio.sleep(1)
        print("Foo done")
    except asyncio.CancelledError:
        print("Foo got canceled")

async def main():
    print("main started")
    task = asyncio.ensure_future(foo())
    task.cancel()
    await asyncio.sleep(2)
    print("main done")

loop = asyncio.get_event_loop()
loop.run_until_complete(main())

>>> main started
>>> main done

So as half-baked workaround every coroutine would need to yield right at the start to ensure it can catch a Cancellation right at the start while the caller

Would this also work for you? My thinking is: if the generator hasn't started there is nothing to cancel.

def my_task_cancel(t):
    try:
        asyncio.cancel(t)
    except TypeError:
        pass

I wish pend_throw() raised a more specific exception in case the generator had not started yet.

No that doesn't work.

Would this also work for you? My thinking is: if the generator hasn't started there is nothing to cancel.

The logic here is wrong. A generator that hasn't been started is already in the uasyncio loop and will therefore be started as soon as the loop gets to it.
As soon as you call

loop.create_task(t)

the generator is out of your control and will get started.

the generator is out of your control and will get started.

You are correct, I was confused about the semantics of pend_throw(). I implemented task cancellation in an old version of uasyncio (years ago!) when pend_throw() didn't exist.

Next attempt:

import uasyncio as asyncio

class MyExc(Exception):
    pass

def my_task_cancel(t):
    try:
        asyncio.cancel(t)
    except TypeError as e:
        try:
            print("Caught <{}>".format(e))
            t.throw(MyExc("Canceling {} synchronously".format(t)))
        except MyExc as e:
            print(e)
    else:
        print("Canceling {} asynchronously".format(t))

async def test1():
    try:
        print("test1.1")
        await asyncio.sleep(1)
        print("test1.2")
        await asyncio.sleep(1)
    except Exception as e:
        print("test1", type(e), e)
        raise

async def test2():
    try:
        print("test2.1")
        await asyncio.sleep(1)
        print("test2.2")
        await asyncio.sleep(1)
    except Exception as e:
        print("test2", e)
        raise

async def dump():
    while True:
        loop = asyncio.get_event_loop()
        print("===== Loop at", loop.time(), "ms, r:", len(loop.runq), "w:", len(loop.waitq))
        if len(loop.runq) == 0 and len(loop.waitq) == 0:
            break
        await asyncio.sleep(0.5)

d = dump()
t1 = test1()
t2 = test2()

async def cancel():
    my_task_cancel(t1)
    my_task_cancel(t2)

loop=asyncio.get_event_loop()
loop.create_task(t1)
loop.create_task(cancel())
loop.create_task(t2)
loop.run_until_complete(d)

Which seems to work in the surface...

$ ./micropython task_cancel_test.py
test1.1
Canceling <generator object 'test1' at b7c74a70> asynchronously
Caught <can't pend throw to just-started generator>
Canceling <generator object 'test2' at b7c74e40> synchronously
===== Loop at 18976639 ms, r: 0 w: 1
===== Loop at 18977141 ms, r: 0 w: 1
test1 <class 'CancelledError'>
===== Loop at 18977645 ms, r: 0 w: 0

However, how does current uasyncio deal with task cancelation when a task is registered with a poller? I recall that dealing with that case required some changes to the event loop core and it wasn't pretty.

I guess you could amend call_soon() to add pending coros to a set, discarding them after they are popped. The cancel function would first check if the coro is in the set of pending coros: if so the coro would be removed from runq and discarded from the set and the function would return.

This presupposes a fix to runq enabling entries to be removed, as per Paul's fix for the cancellation bug. In my opinion Paul's fix is the correct way to ensure tasks get cancelled promptly.

The idea of using a set in this way does break the non-allocating nature of uasyncio.core: perhaps someone can think of a better solution.

@dxxb

@dxxb This is indeed a workaround that will work too. Thanks for that.
Using this for task cancellation could be helpful in lack of a better way:

def cancel(t):
   try:
        asyncio.cancel(t)
    except TypeError as e:
        try:
            print("Caught <{}>".format(e))
            t.throw(MyExc("Canceling {} synchronously".format(t)))
        except MyExc as e:
            print(e)
    else:
        print("Canceling {} asynchronously".format(t))

If generators can't receive a pend_throw before they are started, then what @peterhinch suggests seems to be the cleaner way, finally introducing the fixes to runq and waitq as per Paul's fixes.
That way we'd now get rid of 2 bugs in uasyncio.
The set is a nice workaround but one that would have to be done on uasyncio module level so when messing around there, you could just as well fix timeq module for waitq and deque for runq.

@kevinkk525

I have prototyped this approach in my fast_io fork with some success. I adapted it from my suggestion above so it works with official firmware.

The EventLoop class has a .pending set: task ID's are added to this before the task is placed on any queue. EventLoop already has a .canned set of tasks to be cancelled. cancel() first checks if the task is in .pending: if so it removes it from .pending, adds it to .canned and quits without running pend_throw.

In the EventLoop.run_forever() loop, before a task is moved from any of the queues to runq it is checked to see if it is in .canned: if so it is removed from .canned and the task is thrown away. This is the existing cancellation method. Now, when a task is popped from runq it is checked for membership of .canned: if so it is thrown away. This caters for the case of a task which has been put on runq directly (by yield or sleep(0)) and which hasn't yet run.

This needs further refinement to ensure that a task is not added to .pending when it is rescheduled. I will update fast_io in when I have an acceptable solution. Adding to .pending will perform an additional allocation when a task is first launched.

@kevinkk525 I appreciate you want to use official uasyncio but it shows a way towards a fix. But I do hope someone thinks of a better way...

Your implementation has some "nice" workarounds but the cancel will still fail on a just-started generator as your cancel starts with:

def cancel(coro):
    prev = coro.pend_throw(CancelledError())

But after that the logic seems right.

It's however quite the workaround for a problem that should be fixed in the underlying libraries.
I hope we don't have to wait years for a fix. I'd create PRs for utimeq and deque but I'm afraid that all I could contribute would be copying pfalcons implementation.. So better someone else looks at it.
We need a better cancellation implementation for uasyncio.

@kevinkk525

Thanks for raising this @kevinkk525 and the suggestions from @peterhinch and @dxxb .

I think the best approach here is to just fix pend_throw to work for unstarted coroutines. This way we avoid having to change uasyncio or risk breaking anything or do any additional allocations. I think I see a way to implement this efficiently in objgenerator.c.

@dxxb

That would be great. Thanks.
Basically throwing an exception to the generator directly like @dxxb suggested works exactly in the way one would expect it to work, the generator goes straight to a StopIteration when uasyncio tries to execute it and therefore no code of the generator gets executed.

However the cancellation in uasyncio still has issues (only cancelling after a long yield has finished) and would need work too although sadly it seems that there's little interest in changing uasyncio and the underlying modules.

See #5275

However the cancellation in uasyncio still has issues (only cancelling after a long yield has finished) and would need work too although sadly it seems that there's little interest in changing uasyncio and the underlying modules.

I agree there are lots of things that can be improved, I've got a few on my todo list, but can you provide more information about this specific issue? There are other changes in the works for uasyncio (e.g. #5222).

I'm not aware of any code that could break

My code does, for instance.

I'd like to continue that discussion in your PR instead of in this issue because it is considered closed.

This issue is open and I think discussion as to how to resolve your use case in a way that doesn't break existing code should continue here.

In my opinion it doesn't matter if it breaks code that wasn't CPython compliant anyway

I'll say it again: pend_throw() is a micropython extension one that was designed to behave in a specific way. It is an API, APIs are not broken lightly because people have code that depends on them.

To all the rest of your points: uasyncio is not compatible with CPython's asyncio and was not meant to be. It cannot be made compatible without dropping its main features, that is way it is called uasyncio and not asyncio. For asyncio compatibility a new asyncio module needs to be created.

You figured uasyncio is the closest there is to asyncio but it doesn't quite do what you want (I have been there believe me). So you want to change uasyncio to be more like asyncio because it's soooo close but not quite. But uasyncio is the way it is by design, the author didn't decide to make it gratuitously incompatible, uasyncio is the way it is because it has to run on very memory constrained devices and asyncio would not be able to.

I would like to have a CPython compatible asyncio available for micropython too but uasyncio is what I found and people wrote code that works with it, me included, so incompatible changes to its API should not be made lightly.

No that is not why I opened this issue. I opened this issue because I couldn't prevent a non-started coroutine from running and actually had to let it run until the first yield, which is exactly what you want but exactly the behaviour that was wrong in my opinion (besides the exception from pend_throw) and differently from CPython.

My bad, so you want uasyncio to behave differently to how it legitimately does, please see above for reasons why your use case should not break existing code. I thought we established the following workaround does exactly what you wanted without any changes to micropython core?

def cancel(coro):
    try:
        prev = coro.pend_throw(CancelledError())
        if prev is False:
            _event_loop.call_soon(coro)
    except TypeError as e:
        coro.throw(CancelledError())

If so, your problem can be solved locally for you without breaking uasyncio for other people. Note that the version of cancel() I quoted above should not be added to uasyncio as-is because it doesn't give tasks a chance to handle exceptions when the task has not started.

We could change the signature of cancel(coro) in uasyncio by adding a keyword argument cancel(coro, kill_unstarted=False) so the application can pass True if they want your desired behaviour.

From your #5288 (comment)

Please provide a code example of how you were able to catch all exceptions before the changes in #5242 because I couldn't think of any.

Previously all exceptions accepted by pend_throw() were raised at the location of yield statements so all exceptions could be handled by wrapping them in try: except: finally: blocks.

But judging from your comments, your changes are incompatible to CPython behaviour and therefore shouldn't be implemented. It would make creating code more confusing.

The intent in #5288 is to restore the semantics of pend_throw() to pre-#5275 state to fix the exception handling issue #5275 introduces without losing the refactoring that was part of #5275. It is fixing a problem and CPython compatibility has nothing to do with it.

You have yet to post a minimal example of what actually is broken in your code.

Besides that: uasyncio is not a different asyncio library, it is just a smaller version of asyncio and therefore always has the goal that the features that are implemented are as close to CPython asyncio as possible.

Previously all exceptions accepted by pend_throw() were raised at the location of yield statements so all exceptions could be handled by wrapping them in try: except: finally: blocks.

Well it still does that. The only thing that changed is that pend_throw doesn't throw an Exception on an unstarted generator. Before the PR pend_throw would raise an Exception on an unstarted generator but not throw it when the generator reaches the next yield. It never did that.
Therefore your code must do something like this to be able to run generators to the first yield even when they are canceled:

def cancel(coro):
    try:
        asyncio.cancel(coro)
    except TypeError:
        await asyncio.sleep(0)
        cancel(coro)

So please provide a code example so we can all see what actually breaks.

Besides that: uasyncio is not a different asyncio library, it is just a smaller version of asyncio and therefore always has the goal that the features that are implemented are as close to CPython asyncio as possible.

Years of commit history, issue tracker and forum discussions about uasyncio disagree with your ~~hubris~~ statement. May I suggest their perusal before deciding for others what uasyncio is (or is not) and what its goals are?

uasyncio can be improved and, as you have seen from the beginning of this discussion, people were happy to provide potential solutions to your problem and volunteered, myself included, to work on improving uasyncio. As I tried to explain earlier, when I first started to use uasyncio I wanted it to be more like asyncio and did not understand why it was so annoyingly and artificially incompatible, then after a while the apprentice learned it was so for a reason.

The only thing that changed is that pend_throw doesn't throw an Exception on an unstarted generator.

No, it is not the only thing that changed. I pointed out repeatedly in my comments that now an exception scheduled using pend_throw() in a not-started generator is not raised in the location of a yield statement.

So please provide a code example so we can all see what actually breaks.

For an example one does not need to look further than the test suite for #5275, see 5578182#diff-0854c0e6e091112656606d1ddf368cc2R80

Any task that relies on execution reaching the first yield statement to catch exceptions will behave differently after #5275

class CancelledError(Exception):
	pass

foo = 0

def gen_cancelled():
	global foo
	for i in range(5):
		try:
 			yield i
		except CancelledError:
 			print('Do something before cancellation')
 			foo = 10

# Cancel started generator
foo = 1
g = gen_cancelled()
next(g)
g.pend_throw(CancelledError())
next(g)
assert foo == 10, foo

# Cancel not-started generator
foo = 2
g = gen_cancelled()
g.pend_throw(CancelledError())
try:
	next(g)
except Exception as e:
	print("raised", repr(e))
# AssertionError because the generator didn't get the chance to catch CancelledError()
assert foo == 10, foo

Previously excution would have always reached yield before allowing a pending exception to be raised simply because pend_throw() did not allow to inject an exception unless the yield statement had been reached.

It's self-evident that a script crafted to demonstrate the effects of the PR will do so. The point as I see it is that pend_throw is not CPython compatible. It is for internal use in uasyncio and not intended for use in applications.

Does the PR break existing code which uses uasyncio's provided cancellation or timeout capabilities? And, if so, does the behaviour with the PR differ from that of CPython?

I still don't see where this code should have been broken because the line "g.pend_throw(CancelledError())" would have raised a TypeError before and you would have never gotten to the part where the generator executes until the yield.

Do you have actual code that you actually tested to be broken that you can present?

Previously excution would have always reached yield before allowing a pending exception to be raised simply because pend_throw() did not allow to inject an exception unless the yield statement had been reached.

Yeah but simply because one had no means of cancelling a non-started generator because calling asyncio.cancel() would raise a TypeError. That means your code would rely on ignoring that exception and scheduling a cancel later when the coro has run once. So please provide a real code example.. (a real one, not a theoretical construct. Part of your code that is now broken)

@peterhinch

It's self-evident that a script crafted to demonstrate the effects of the PR will do so.

And yet it shows that now exceptions can escape handling by the generator which is the root of the issue.

It is for internal use in uasyncio and not intended for use in applications.

@peterhinch, pend_throw() is a python function (no leading _) so it is prefectly reasonable to assume it is public. But more importantly previous pend_throw() behaviour is required to ensure generators behave correctly when exceptions are injected which is useful beyond uasyncio.

The point as I see it is that pend_throw is not CPython compatible.

As I pointed out countless times by now this is irrelevant because CPython does not expose or ducument pend_throw(), and yet...

And, if so, does the behaviour with the PR differ from that of CPython?

... the behaviour of cancel() in CPython (Python 3.7.4) does not agree with CPython's own documentation of cancel(). In order to implement the behaviour of cancel(), described by CPython's own documentation, the guaranteee that all exceptions injected by pend_throw() are handled by the generator is required. This guarantee was present in previous behaviour and has been removed by 5578182

Does the PR break existing code which uses uasyncio's provided cancellation or timeout capabilities?

Yes, it breaks tasks (generators) written to assume they will be always given a chance to catch cancellation exceptions and the issues it creates may not be immediately obvious because it is timing dependent.

@dxxb

@dxxb I agree: pend_throw should have a leading _. It has been pointed out that it is an internal function, but the name should provide a further hint.

Using undocumented internal features in applications carries hazards: the functionality may change or the feature may be removed if it is no longer required to support a future version of uasyncio.

@dxxb

@dxxb I agree: pend_throw should have a leading _. It has been pointed out that it is an internal function, but the name should provide a further hint.

I am sorry but the cat is out of the bag. The function has been public for a long time and it makes sense for people to use it and expect it isn't broken.

However, I would be perfectly happy to make changes to existing code I maintain if it made sense moving forward, but the changes we are talking about have a negative impact on the correct functioning of uasyncio.

I am sorry but the cat is out of the bag. The function has been public for a long time and it makes sense for people to use it and expect it isn't broken.

Just a sidenote, but I really don't think MicroPython's intent is generally adhering this principle, see e.g. second paragraph of the main README:

WARNING: this project is in beta stage and is subject to changes of the code-base, including project-wide name changes and API changes.

There's a whole lot of functions in MicroPython, both in Python and C code, which have been publicly available for a long time but still remain implementation details and hence usually are subject to change. So they should be treated as 'you can use it at your own risk'. I understand that's not always ideal, I've experienced this myself a number of times, but I knew on beforehand this was bound to happen.

Just a sidenote, but I really don't think MicroPython's intent is generally adhering this principle, see e.g. second paragraph of the main README

Acknowledged, although I have the feeling that line is may be a leftover from earlier stages of the project. Even considering the .pend_throw() subject to changes, my issue with the changes we are talking about is two-fold: one is with process because legitimate concerns were raised and less invasive solutions proposed but the more invasive and breaking change was rushed through (which I now realised may be part of a not-so-well-publicized effort to replace uasyncio) and corrective action was not taken after its issues were highlighted, the other is with the fact that these changes (as they stand right now) IMO prevent implementing a well-behaved async scheduler that fits the same memory footprint uasyncio was designed for.

py/objgenerator: Allow pend_throw to an unstarted generator.

The new uasyncio v3 makes this issue obsolete.

Merge pull request micropython#5242 from lesamouraipourpre/patch-2

py/objgenerator: Allow pend_throw to an unstarted generator.

Navigation Menu

Repository navigation

Cancelling coroutines: can't pend throw to just-started generator. #5242

30 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Cancelling coroutines: can't pend throw to just-started generator. #5242

Description

Activity

dpgeorge commented on Oct 22, 2019

kevinkk525 commented on Oct 22, 2019

dxxb commented on Oct 22, 2019

kevinkk525 commented on Oct 22, 2019

dxxb commented on Oct 22, 2019

peterhinch commented on Oct 22, 2019

kevinkk525 commented on Oct 22, 2019

peterhinch commented on Oct 22, 2019

kevinkk525 commented on Oct 23, 2019

jimmo commented on Oct 29, 2019

kevinkk525 commented on Oct 29, 2019

jimmo commented on Oct 29, 2019

30 remaining items

dxxb commented on Nov 4, 2019

kevinkk525 commented on Nov 4, 2019

dxxb commented on Nov 4, 2019

peterhinch commented on Nov 5, 2019

kevinkk525 commented on Nov 5, 2019

dxxb commented on Nov 5, 2019

peterhinch commented on Nov 6, 2019

dxxb commented on Nov 6, 2019

stinos commented on Nov 6, 2019

dxxb commented on Nov 6, 2019

dpgeorge commented on Jul 13, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions