classification
Title: crash if asyncio is used before and after re-initialization if using python embedded in an application
Type: crash Stage: patch review
Components: asyncio Versions: Python 3.9
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: HiassofT, asvetlov, benjamin-sch, miss-islington, vstinner, yselivanov
Priority: normal Keywords: patch

Created on 2021-09-22 08:32 by benjamin-sch, last changed 2021-10-08 08:55 by vstinner.

Pull Requests
URL Status Linked Edit
PR 28796 merged HiassofT, 2021-10-07 13:28
PR 28815 merged miss-islington, 2021-10-07 22:47
PR 28816 merged miss-islington, 2021-10-07 22:47
Messages (8)
msg402412 - (view) Author: Benjamin Schiller (benjamin-sch) Date: 2021-09-22 08:32
We have embedded Python in our application and we deinitialize/initialize the interpreter at some point of time. If a simple script with a thread that sleeps with asyncio.sleep is loaded before and after the re-initialization, then we get the following assertion in the second run of the python module:

"Assertion failed: Py_IS_TYPE(rl, &PyRunningLoopHolder_Type), file D:\a\1\s\Modules_asynciomodule.c, line 261"

Example to reproduce this crash: https://github.com/benjamin-sch/asyncio_crash_in_second_run
msg402469 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2021-09-22 19:57
I guess the fix requires switching C Extension types from static to heap for _asyncio module.
It is possible for sure but requires a non-trivial amount of work.

We need a champion for the issue.
msg402905 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-29 20:41
> "Assertion failed: Py_IS_TYPE(rl, &PyRunningLoopHolder_Type), file D:\a\1\s\Modules_asynciomodule.c, line 261"

The _asyncio extension uses static types and doesn't implement the multiphase initialization API. It doesn't work well with your "deinitialize/initialize the interpreter" use case.

The _asyncio extension should use heap types and the multiphase init API.
msg403407 - (view) Author: Matthias Reichl (HiassofT) * Date: 2021-10-07 13:37
We were hitting the same issue in kodi, which uses embedded sub-interpreters to run python addons, after one of the addons was switched to asyncio.

The immediate cause of the assertion failure is a use-after-free issue from the running loop holder cache:

When the running loop holder is deallocated (which happens eg on interpreter shutdown) cached_running_holder holds a dangling pointer.

A subsequent call to get_running_loop() may then pick that up and boom.

While probably not a full fix for this issue I think it would be good to fix the use-after-free problem - there could be other code paths that lead to this situation. I've created a github pull request for that
msg403445 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-10-07 22:47
New changeset 392a89835371baa0fc4bf79ae479abb80661f57d by Matthias Reichl in branch 'main':
 bpo-45262, asyncio: Fix cache of the running loop holder (GH-28796)
https://github.com/python/cpython/commit/392a89835371baa0fc4bf79ae479abb80661f57d
msg403448 - (view) Author: miss-islington (miss-islington) Date: 2021-10-07 23:14
New changeset 87f0156a229e4cda92ad8e50645c5a71030caf7c by Miss Islington (bot) in branch '3.9':
bpo-45262, asyncio: Fix cache of the running loop holder (GH-28796)
https://github.com/python/cpython/commit/87f0156a229e4cda92ad8e50645c5a71030caf7c
msg403462 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-10-08 08:55
Is Matthias Reichl's fix enough to use asyncio in a subinterpreter?
msg403463 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-10-08 08:55
New changeset 6846d6712a0894f8e1a91716c11dd79f42864216 by Miss Islington (bot) in branch '3.10':
bpo-45262, asyncio: Fix cache of the running loop holder (GH-28796) (GH-28816)
https://github.com/python/cpython/commit/6846d6712a0894f8e1a91716c11dd79f42864216
History
Date User Action Args
2021-10-08 08:55:45vstinnersetmessages: + msg403463
2021-10-08 08:55:30vstinnersetmessages: + msg403462
2021-10-07 23:14:11miss-islingtonsetmessages: + msg403448
2021-10-07 22:47:42miss-islingtonsetpull_requests: + pull_request27134
2021-10-07 22:47:15miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request27133
2021-10-07 22:47:02vstinnersetmessages: + msg403445
2021-10-07 13:37:57HiassofTsetnosy: vstinner, asvetlov, yselivanov, benjamin-sch, HiassofT
messages: + msg403407
2021-10-07 13:28:44HiassofTsetkeywords: + patch
nosy: + HiassofT

pull_requests: + pull_request27124
stage: patch review
2021-09-29 20:41:07vstinnersetnosy: + vstinner
messages: + msg402905
2021-09-22 19:57:30asvetlovsetmessages: + msg402469
2021-09-22 08:32:26benjamin-schcreate