Skip to content

Conversation

@smurfix
Copy link
Contributor

@smurfix smurfix commented Jun 20, 2022

This backports 3.11's TaskGroup class to uasyncio.

I don't care much about the except * stuff, not in µPy context anyway, but sane error recovery is crucial and taskgroups make this job a whole lot easier, not to mention much less bug prone. (Writing from a lot of Trio and anyio experience here.)

Bottom line: I refuse to write async code without using taskgroups. So here you are.

TODO: Write a couple of tests.

Closes #8508.

@smurfix smurfix force-pushed the tg branch 2 times, most recently from 5a46a80 to b5723ea Compare June 20, 2022 20:08
@smurfix
Copy link
Contributor Author

smurfix commented Jun 20, 2022

Sigh. It's not my commit message that's failing the test!

@dpgeorge dpgeorge added the extmod Relates to extmod/ directory in source label Jun 21, 2022
@dpgeorge
Copy link
Member

Wow, thanks for this! It's very nice to see that it can be implemented in pure Python. I don't know anything about TaskGroup so will need to learn how it works.

@dpgeorge
Copy link
Member

It's not my commit message that's failing the test!

You're right. That CI check will need fixing so it allows "Revert" commit messages.

@dpgeorge
Copy link
Member

It's not my commit message that's failing the test!

You're right. That CI check will need fixing so it allows "Revert" commit messages.

Actually the CI check is working OK, it's only supposed to check the commit messages that are unique to the PR.

If you rebase (and force push) on latest master it will hopefully get the CI green.

@smurfix
Copy link
Contributor Author

smurfix commented Jun 21, 2022

Pushed. Writing tests, found a significant bug (related to cancellation of course). Investigating.

@smurfix smurfix force-pushed the tg branch 8 times, most recently from 1a0dffd to 7d20532 Compare June 22, 2022 19:29
@smurfix
Copy link
Contributor Author

smurfix commented Jun 22, 2022

Huh. I have no idea where those test failures are coming from.

@dlech
Copy link
Contributor

dlech commented Jun 22, 2022

The CI logs say error is:

+micropython-coverage: ../../py/emitnative.c:2806: emit_native_raise_varargs: Assertion `n_args == 1' failed.

It looks like the bytecode emitter allows 2 args but the native emitter only allows 1.

@smurfix
Copy link
Contributor Author

smurfix commented Jun 22, 2022

It looks like the bytecode emitter allows 2 args but the native emitter only allows 1.

Well, yeah, I saw that line, but what has that got to do with implementing __hash__ and/or how do I fix that? I didn't touch no emitters … so either I'm doing something really dumb here, or I found a bug which I'm not qualified to fix, esp. since the code works on my system(s).

@dlech
Copy link
Contributor

dlech commented Jun 23, 2022

Assuming you are using the unix port, to reproduce locally, try make -C ports/unix test_full or run micropython with -X emit=native.

There are a number of tests that are disabled when emit=native because of raise_varargs already, basics/try_reraise.py, basics/try_reraise2 and basics/try_finally_return2.py to name a few (search for raise_varargs in tests/run-tests.py).

I suspect you have written some Python code similar to one of these that causes the same error.

@peterhinch
Copy link
Contributor

I'm running this on a Pyboard 1.1 - TaskGroups are a very nice feature.

Something Google seems loath to divulge is how to access return values from the members of a group (assuming they terminate normally).

@smurfix
Copy link
Contributor Author

smurfix commented Jun 25, 2022

@peterhinch Glad that my code is of use. I'll try to fix up the CI errors shortly; battling with Covid aftereffects, so if somebody else wants to fix up this PR, feel free.

You need to return any values explicitly, e.g. by setting an object attribute to it or queuing the value or whatever.

@peterhinch
Copy link
Contributor

Sorry you're unwell - I hope you feel better soon.

Writing tests, found a significant bug (related to cancellation of course). Investigating.

Is this still outstanding or is the code ready for review?

I very much hope this is implemented: in many applications it's a big improvement over gather.

@smurfix
Copy link
Contributor Author

smurfix commented Jun 26, 2022

Is this still outstanding or is the code ready for review?

No, that's done. The workaround I implemented admittedly isn't particularly clean, but it gets the job done and doesn't alter non-taskgroup code (as that might introduce bugs or slowdowns).

@peterhinch
Copy link
Contributor

@dpgeorge @jimmo This is to advocate for TaskGroup with a real world example where they fit the bill perfectly.

Consider a communication link between two peers using an unreliable medium such as WiFi. This may be near the limit of range and the link can fail in a variety of ways. The simplest way to recover is the "belt and braces" approach: if a peer encounters an unrecoverable error, it takes the link down for a period long enough for the other peer also to suffer an unrecoverable error. That way both peers start from a "power up" state.

A TaskGroup might then comprise:

  • Task trying to keep the WiFi available through brief outages.
  • Task waiting on received messages.
  • Task pinging the other peer and checking a response.
  • Intermittently a task is added to the group. It sends a message, awaits an ack, and terminates.

If any of these experience an error that cannot be handled locally, the exception is passed up to be handled by the task that created the group. When this occurs, every task in the group terminates in an orderly way, running cleanup code in finally clauses or via context managers.

The TaskGroup is ideal for this - and in my testing works fine :) I rest my case...

@smurfix
Copy link
Contributor Author

smurfix commented Jun 27, 2022

@peterhinch Exactly. Unstructured tasks (like asyncio's baseline tasks) mean that you have to keep track of what's still running and what might have to be cancelled and/or restarted on your own. Code doing that tends to contain a heap of bugs you can't test for, and basically doesn't scale.

In contrast, when a taskgroup's async context manager has ended you know that there's no dangling tasks, unprocessed finally clauses, unreported exceptions, or similar nonsense around; you can proceed with confidence. When (not if) the framework ensures that whole classes of errors can't happen, you no longer have to test for them, or even think about them.

The latter point is much more powerful than you'd assume at first glance.

@smurfix
Copy link
Contributor Author

smurfix commented Apr 11, 2025

Updated to current master.

The CPython discussion on what to do about the Taskgroup.cancel method has stalled for now (and mainly centered on the name of the feature, as opposed to how it's supposed to work). That doesn't surprise me because the semantics of cancellation are somewhat murky in asyncio, but as cancelling all tasks in a taskgroup is something reasonable to do …

+mpy-cross: ../py/emitnative.c:3010: emit_native_raise_varargs: Assertion n_args == 1' failed.`

Umm, that used not to happen AFAIR?

It should be that the scheduler always iterates the singleton immediately

Well, it appears not to. In any case, by its nature async code is async: nothing prevents me from calling a bare asyncio.sleep and then doing something else before using the result in an await. Thus we shouldn't depend on that whether or not it's supposed to apply in the standard case.

    async def not_too_fast(delay, proc):
        dly = asyncio.sleep(delay)
        res = await proc()
        await dly
        return res

In MicroPython this is a nicely concise way of saying "give me the result of proc in exactly two seconds, except when proc takes longer than that`. (This doesn't work the same way on CPython, but not by design AFAICT.)

@smurfix
Copy link
Contributor Author

smurfix commented Oct 8, 2025

dpgeorge added this to the release-1.27.0 milestone last month

That would be superb. Please shout if I can help, e.g. by rebasing things onto the main branch.

This patch adds a mostly-compatible backport of CPython 3.11's
`TaskGroup` class to uasyncio.

Also, there is a new `run_server` method in uasyncio/stream.py which
supports task groups and doesn't run in the background (no need).

TODO: Write some more tests.

Closes micropython#8508.

Signed-off-by: Matthias Urlichs <[email protected]>
The C implementation (_uasyncio.Task) was not.

Signed-off-by: Matthias Urlichs <[email protected]>
Tasks can be cancelled before they have an opportunity to run.
Handling this situation is non-trivial in MicroPython's context.

Signed-off-by: Matthias Urlichs <[email protected]>
If two tasks try to start sleeping at the same time,
the SingletonGenerator will be in use, thus we need
to allocate a new temporary handler on the heap.

As the singleton isn't that single any more, it gets renamed.

Found by taskgroup test micropython#12.

Signed-off-by: Matthias Urlichs <[email protected]>
This fixes some edge cases with cancellation.

Signed-off-by: Matthias Urlichs <[email protected]>
still failing, though

Signed-off-by: Matthias Urlichs <[email protected]>
Rudimentary but works.

Signed-off-by: Matthias Urlichs <[email protected]>
If there's an exception (or more) *and* a cancellation in a taskgroup,
the cancellation must be ignored.

Signed-off-by: Matthias Urlichs <[email protected]>
This mirrors Python change 594c369.

Signed-off-by: Matthias Urlichs <[email protected]>
Cancelling a taskgroup that's no longer active should not do anything.

Signed-off-by: Matthias Urlichs <[email protected]>
`run_server` is not in CPython. We like to avoid that.

Signed-off-by: Matthias Urlichs <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

extmod Relates to extmod/ directory in source

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PEP 654: Exception groups and except *, leading to asyncio TaskGroup

10 participants