gh-128002: fix asyncio.all_tasks against concurrent deallocations of tasks#128541
gh-128002: fix asyncio.all_tasks against concurrent deallocations of tasks#128541kumaraditya303 merged 4 commits intopython:mainfrom
asyncio.all_tasks against concurrent deallocations of tasks#128541Conversation
kumaraditya303
requested review from
1st1,
asvetlov,
ericsnowcurrently and
willingc
as code owners
colesbury
left a comment
colesbury
left a comment
There was a problem hiding this comment.
This doesn't look right to me. The object can still be part way through a dealloc call when a stop the world call happens.
Can you provide more details about what bug or crash you are trying to fix?
Please see #128002 (comment) The bug is that the linked list holds borrowed references to tasks so it is possible that task is concurrently deallocated while it gets added to tasks list. |
|
Crash backtrace: DetailsObjects/object.c:578: PyObject_CallFinalizerFromDealloc: Assertion failed: PyObject_CallFinalizerFromDealloc called on object with a non-zero refcount object address : 0x7fb3781e1820 Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed Thread 0x00007fb34edfd6c0 (most recent call first): Current thread 0x00007fb34fdff6c0 (most recent call first): Thread 0x00007fb3542ff6c0 (most recent call first): Thread 0x00007fb3523fe6c0 (most recent call first): Thread 0x00007fb3a66ebf40 (most recent call first): == Tests result: FAILURE == 1 test failed: 50 tests OK. Total duration: 5 min 30 sec |
|
Do you have a command to reproduce the crash? You wrote that:
There are two things that confuse me:
Additionally, the way this linked list works worries me:
Anyways, I think it may be worth trying to match the weakref implementation since we know that it works. |
|
From looking at the crash backtrace, I think the important thing here would be to use Something like: llist_for_each_safe(node, &state->asyncio_tasks_head) {
TaskObj *task = llist_data(node, TaskObj, task_node);
if (_Py_TryIncref(task)) {
if (_PyList_AppendTakeRef(tasks, (PyObject *)task) < 0) {
...
}
}
} |
Running |
So I think the following changes need to be made:
Did I get it correctly? Aside, thanks for your explanation! I didn't know that it was possible that stop the world could happen while a deallocator is running concurrently. |
|
Yes, but on second thought let's keep the |
kumaraditya303
changed the title
asyncio.all_tasksasyncio.all_tasks against concurrent deallocations of tasks
3 participants
Uh oh!
There was an error while loading. Please reload this page.