Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

asyncio: MicroPython vs. CPython create_task() garbage collection behavior #12299

Open
paravoid opened this issue Aug 25, 2023 · 5 comments
Open

Comments

@paravoid
Copy link

(Not sure if I should label this a docs bug, feature request, or discussion. Apologies if I missed the mark.)

CPython exhibits a bit of a counterintuitive behavior, which requires users to store the result of asyncio.create_task(), otherwise face the danger of the task being garbage collected.

Will McGugan of Textual blogged about this here:
https://textual.textualize.io/blog/2023/02/11/the-heisenbug-lurking-in-your-async-code/

Vincent Bernat reported this as a documentation bug against CPython, python/cpython#88831 last year, and subsequently the CPython documentation was adjusted to say:

Important
Save a reference to the result of this function, to avoid a task disappearing mid-execution. The event loop only keeps weak references to tasks. A task that isn’t referenced elsewhere may get garbage collected at any time, even before it’s done. For reliable “fire-and-forget” background tasks, gather them in a collection:

background_tasks = set()

for i in range(10):
    task = asyncio.create_task(some_coro(param=i))

    # Add task to the set. This creates a strong reference.
    background_tasks.add(task)

    # To prevent keeping references to finished tasks forever,
    # make each task remove its own reference from the set after
    # completion:
    task.add_done_callback(background_tasks.discard)

Is MicroPython exhibiting the same behavior? If so, should the documentation be adjusted?

  1. The MicroPython documentation provides an example that does not store a reference, basically what CPython warns users not to do. Is this a misleading example or a non-issue in MicroPython? Should the example be CPython-friendly anyway though? Should the documentation explain this difference?
  2. Even if one wanted to be better-safe-than-sorry and write portable code, the CPython-recommended code above does not work under MicroPython, resulting in a TypeError: unsupported type for __hash__: 'Task'. Should __hash__ be added for Tasks? Or should the documentation mention some other way to do this?

Not a MicroPython bug per se, but also worth mentioning: @peterhinch's (excellent!) async tutorial also has examples where Tasks are not stored. At one point it's mentioned that "[t]he .create_task method returns the Task instance which may be saved for status checking or cancellation" (emphasis mine). May, or should?

@peterhinch
Copy link
Contributor

peterhinch commented Aug 25, 2023

My understanding of the code is that MP does not suffer from this behaviour. All tasks are put on the task queue (code) which provides a reference protecting the task from GC.

Confirmation from @dpgeorge or @jimmo would be good.

For the background_tasks workround to run under MP, tasks would have to be hashable and .add_done_callback would have to be implemented. Assuming that the bug is restricted to CPython, the practical portable workround would be to write code which only implemented the background_tasks fix if running under CPython.

It would be good if this bug in CPython's asyncio were fixed. We can but hope.

@peterhinch
Copy link
Contributor

I have added this note to the tutorial pending definitive confirmation from the maintainers.

@jimmo
Copy link
Member

jimmo commented Aug 30, 2023

Is this a misleading example or a non-issue in MicroPython?

It is both of these things :) We should be providing examples that are also good/valid CPython, but yes, MicroPython does not have this problem because of what Peter said, (and because it does not support weak references).

Related to this, CPython's reference counting also enables CPython to do immediate detection of un-waited tasks when the task goes out of scope.

resulting in a TypeError: unsupported type for hash: 'Task'. Should hash be added for Tasks?

Tasks do implement __hash__, but if you're using the optimised Task implementation, I think this only got implemented in ~May when we added implicit hash by default for all built-in types. So you might find this is already fixed in the latest builds (and if it isn't please let us know because that would be surprising).

@peterhinch
Copy link
Contributor

@jimmo Thanks for that. I will amend the sample scripts in the tutorial to save task references for the CPython workround.

I can confirm that tasks can now be added to a set.

@paravoid
Copy link
Author

Status update, as of today, 2024-01-15, tested with MicroPython v1.22.1:

  • Tasks are now hashable and able to be added to a set(), as also reported above.
  • task.add_done_callback(background_tasks.discard) (from the CPython doc) does not work, as add_done_callback() is not yet implemented.
  • https://docs.micropython.org/en/latest/library/asyncio.html has not been adjusted to be CPython-friendly, by providing example code that stores tasks to a set.
  • @peterhinch's async tutorial has been updated to add a CPython compatibility note, also linking to this issue (thanks!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants