Could this change be used to allow additional states so that this implementation could be used to implement shield (e.g. when using an integer for the state instead of False,None,True)? #5928
(Just a quick thought without having gone through any possible implementations)

Could this change be used to allow additional states so that this implementation could be used to implement shield (e.g. when using an integer for the state instead of False,None,True)?

Yes possibly. The big issue is whether the main dispatch loop needs to handle these special cases/states or not. One nice thing about the patch in this PR is that the main dispatch loop is unchanged, it's still either t.coro.send(None) or t.coro.throw(exc).

shield() only needs modifications in task.py, the main loop is not touched. It just needs a place to store the shielded state but upon taking a closer look, I realize that it won't work with this PR because

#  Allocated head of linked list of Tasks waiting on completion of this task.
self.state = TaskQueue()

A shielded task would have other Tasks waiting on the completion, so it would need this state allocation and can't use it for the shielded state anyway. So we're back at where we started..

A shielded task would have other Tasks waiting on the completion, so it would need this state allocation and can't use it for the shielded state anyway.

It could possibly use a tuple to indicate shielded:

self.state = (TaskQueue(),)

That uses memory but only if the code uses the shield feature.

But wouldn't that require that the rest of the code always checks if that argument is a tuple or just a simple TaskQueue? That would require more code space and execution time even if it saves a bit of RAM.

But wouldn't that require that the rest of the code always checks if that argument is a tuple or just a simple TaskQueue? That would require more code space and execution time even if it saves a bit of RAM.

Yes it would, but there could be only a few such checks. It would need investigation.

The reason to be careful with RAM here is because if there are 100's of tasks then the memory usage can grow quite large if each task uses more memory.

I can also confirm that this fix has solved the issue we've seen on our device. Thanks a lot for the quick fix!

A note on the fix.. We've been using the "coro" attribute on the task's object to implement a tracer that measures the execution of each task automatically.

For this we need write access to the coro attribute. I saw that in the C-code of the asyncio's task, write access to this attribute has been deleted... Would you accept a PR that would add / allow writing the coro attribute again (as it was before)?

Bascially it would boil down to this diff

diff --git a/extmod/moduasyncio.c b/extmod/moduasyncio.c
index 9717e3856..50a5e5855 100644
--- a/extmod/moduasyncio.c
+++ b/extmod/moduasyncio.c
@@ -243,7 +243,10 @@ STATIC void task_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest) {
         }
     } else if (dest[1] != MP_OBJ_NULL) {
         // Store
-        if (attr == MP_QSTR_data) {
+        if (attr == MP_QSTR_coro) {
+            self->coro = dest[1];
+            dest[0] = MP_OBJ_NULL;
+        } else if (attr == MP_QSTR_data) {
             self->data = dest[1];
             dest[0] = MP_OBJ_NULL;
         } else if (attr == MP_QSTR_state) {

Bascially it would boil down to this diff

I'd really prefer not to make coro writable, or even to expose it at all to user code. Can you please elaborate on how you use it? It could be that it does make sense to expose it.

@hoihu actually, please open an issue to discuss the above point about making coro writable, so it doesn't get lost in this closed/merged PR.