From ec21ef8989390d523446bf2a8db862fc9258089b Mon Sep 17 00:00:00 2001 From: Savannah Ostrowski Date: Tue, 9 Apr 2024 02:48:33 +0000 Subject: [PATCH 01/11] Add conditional block to check if modules asyncio --- Modules/_asynciomodule.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 29246cfa6afd00..a31d53accca5a4 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -1602,8 +1602,16 @@ FutureIter_dealloc(futureiterobject *it) { PyTypeObject *tp = Py_TYPE(it); asyncio_state *state = get_asyncio_state_by_def((PyObject *)it); - PyObject_GC_UnTrack(it); - tp->tp_clear((PyObject *)it); + + assert(_PyType_HasFeature((PyTypeObject *)tp, Py_TPFLAGS_HEAPTYPE)); + + PyHeapTypeObject *ht = (PyHeapTypeObject*)tp; + PyObject *module = ht->ht_module; + + if (module && _PyModule_GetDef(module) == &_asynciomodule) { + PyObject_GC_UnTrack(it); + tp->tp_clear((PyObject *)it); + } if (state->fi_freelist_len < FI_FREELIST_MAXLEN) { state->fi_freelist_len++; From ca1c043aa339c0a3eb9e2a791d10d546d1f16814 Mon Sep 17 00:00:00 2001 From: Savannah Ostrowski Date: Wed, 10 Apr 2024 03:15:01 +0000 Subject: [PATCH 02/11] remove get_asyncio_state_by_def --- Modules/_asynciomodule.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index a31d53accca5a4..525ddc046cbced 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -1601,23 +1601,25 @@ static void FutureIter_dealloc(futureiterobject *it) { PyTypeObject *tp = Py_TYPE(it); - asyncio_state *state = get_asyncio_state_by_def((PyObject *)it); assert(_PyType_HasFeature((PyTypeObject *)tp, Py_TPFLAGS_HEAPTYPE)); PyHeapTypeObject *ht = (PyHeapTypeObject*)tp; PyObject *module = ht->ht_module; + asyncio_state *state = NULL; if (module && _PyModule_GetDef(module) == &_asynciomodule) { + state = get_asyncio_state(module); PyObject_GC_UnTrack(it); tp->tp_clear((PyObject *)it); } - if (state->fi_freelist_len < FI_FREELIST_MAXLEN) { + if (state && state->fi_freelist_len < FI_FREELIST_MAXLEN) { state->fi_freelist_len++; it->future = (FutureObj*) state->fi_freelist; state->fi_freelist = it; } + else { PyObject_GC_Del(it); Py_DECREF(tp); From 0845b28dbb980747a96eb79d6567e55bd33e17f6 Mon Sep 17 00:00:00 2001 From: Savannah Ostrowski Date: Thu, 11 Apr 2024 00:04:22 +0000 Subject: [PATCH 03/11] Always untrack and clear --- Modules/_asynciomodule.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 525ddc046cbced..d643ef1be1e27e 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -1608,10 +1608,11 @@ FutureIter_dealloc(futureiterobject *it) PyObject *module = ht->ht_module; asyncio_state *state = NULL; + PyObject_GC_UnTrack(it); + tp->tp_clear((PyObject *)it); + if (module && _PyModule_GetDef(module) == &_asynciomodule) { state = get_asyncio_state(module); - PyObject_GC_UnTrack(it); - tp->tp_clear((PyObject *)it); } if (state && state->fi_freelist_len < FI_FREELIST_MAXLEN) { @@ -1619,7 +1620,6 @@ FutureIter_dealloc(futureiterobject *it) it->future = (FutureObj*) state->fi_freelist; state->fi_freelist = it; } - else { PyObject_GC_Del(it); Py_DECREF(tp); From 297d8dd3c79aad4f59676bbd4f4381cd32f148a7 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sat, 13 Apr 2024 18:59:27 +0000 Subject: [PATCH 04/11] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2024-04-13-18-59-25.gh-issue-115874.c3xG-E.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-04-13-18-59-25.gh-issue-115874.c3xG-E.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-04-13-18-59-25.gh-issue-115874.c3xG-E.rst b/Misc/NEWS.d/next/Core and Builtins/2024-04-13-18-59-25.gh-issue-115874.c3xG-E.rst new file mode 100644 index 00000000000000..b6d8a4d89ba342 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-04-13-18-59-25.gh-issue-115874.c3xG-E.rst @@ -0,0 +1 @@ +Fixed a possible segfault during garbage collection of `_asyncio.FutureIter` objects From 803c3205af67847e0f9687a54117f4ed019bc4f7 Mon Sep 17 00:00:00 2001 From: Savannah Ostrowski Date: Sat, 13 Apr 2024 19:04:59 +0000 Subject: [PATCH 05/11] fix backticks --- .../2024-04-13-18-59-25.gh-issue-115874.c3xG-E.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-04-13-18-59-25.gh-issue-115874.c3xG-E.rst b/Misc/NEWS.d/next/Core and Builtins/2024-04-13-18-59-25.gh-issue-115874.c3xG-E.rst index b6d8a4d89ba342..5a6fa4cedd9fe4 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2024-04-13-18-59-25.gh-issue-115874.c3xG-E.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2024-04-13-18-59-25.gh-issue-115874.c3xG-E.rst @@ -1 +1 @@ -Fixed a possible segfault during garbage collection of `_asyncio.FutureIter` objects +Fixed a possible segfault during garbage collection of ``_asyncio.FutureIter`` objects From 9b78e149751146c131450c991a0ffe0b0fab651a Mon Sep 17 00:00:00 2001 From: Savannah Ostrowski Date: Sat, 13 Apr 2024 19:56:21 +0000 Subject: [PATCH 06/11] Add comment to reference issue --- Modules/_asynciomodule.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 0c79093d534cfe..8a38717de6bce0 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -1604,6 +1604,7 @@ FutureIter_dealloc(futureiterobject *it) assert(_PyType_HasFeature((PyTypeObject *)tp, Py_TPFLAGS_HEAPTYPE)); + PyHeapTypeObject *ht = (PyHeapTypeObject*)tp; PyObject *module = ht->ht_module; asyncio_state *state = NULL; @@ -1611,6 +1612,9 @@ FutureIter_dealloc(futureiterobject *it) PyObject_GC_UnTrack(it); tp->tp_clear((PyObject *)it); + // GH-115874: We can't use PyType_GetModuleByDef here as the type might have + // already been cleared. This is also why we must check if ht_module != NULL. + // Subclasses also cannot make use of the free list. if (module && _PyModule_GetDef(module) == &_asynciomodule) { state = get_asyncio_state(module); } From 9070afc7814a04ba0ddb46a037b82edb5ec23711 Mon Sep 17 00:00:00 2001 From: Savannah Ostrowski Date: Tue, 16 Apr 2024 01:44:21 +0000 Subject: [PATCH 07/11] Remove newline --- Modules/_asynciomodule.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 8a38717de6bce0..9c9120e8974746 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -1604,7 +1604,6 @@ FutureIter_dealloc(futureiterobject *it) assert(_PyType_HasFeature((PyTypeObject *)tp, Py_TPFLAGS_HEAPTYPE)); - PyHeapTypeObject *ht = (PyHeapTypeObject*)tp; PyObject *module = ht->ht_module; asyncio_state *state = NULL; From 36c960b1a53a0129fcbefb3c18e4722fbd5801ec Mon Sep 17 00:00:00 2001 From: Savannah Ostrowski Date: Wed, 17 Apr 2024 09:36:40 -0700 Subject: [PATCH 08/11] Update Modules/_asynciomodule.c Co-authored-by: Brandt Bucher --- Modules/_asynciomodule.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 9c9120e8974746..0308d7eac33528 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -1604,8 +1604,7 @@ FutureIter_dealloc(futureiterobject *it) assert(_PyType_HasFeature((PyTypeObject *)tp, Py_TPFLAGS_HEAPTYPE)); - PyHeapTypeObject *ht = (PyHeapTypeObject*)tp; - PyObject *module = ht->ht_module; + PyObject *module = ((PyHeapTypeObject*)tp)->ht_module; asyncio_state *state = NULL; PyObject_GC_UnTrack(it); From 69856ece7e471f7c029cfc2daf3a269604bb4599 Mon Sep 17 00:00:00 2001 From: Savannah Ostrowski Date: Wed, 17 Apr 2024 09:38:18 -0700 Subject: [PATCH 09/11] Update Modules/_asynciomodule.c Co-authored-by: Brandt Bucher --- Modules/_asynciomodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 0308d7eac33528..165b0a9a197dd8 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -1602,7 +1602,7 @@ FutureIter_dealloc(futureiterobject *it) { PyTypeObject *tp = Py_TYPE(it); - assert(_PyType_HasFeature((PyTypeObject *)tp, Py_TPFLAGS_HEAPTYPE)); + assert(_PyType_HasFeature(tp, Py_TPFLAGS_HEAPTYPE)); PyObject *module = ((PyHeapTypeObject*)tp)->ht_module; asyncio_state *state = NULL; From 7af78114732daff97585cb71d9050a65f8cdca75 Mon Sep 17 00:00:00 2001 From: Savannah Ostrowski Date: Thu, 18 Apr 2024 02:39:13 +0000 Subject: [PATCH 10/11] Update comment for clarity --- Modules/_asynciomodule.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 165b0a9a197dd8..ad95966800613a 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -1611,8 +1611,9 @@ FutureIter_dealloc(futureiterobject *it) tp->tp_clear((PyObject *)it); // GH-115874: We can't use PyType_GetModuleByDef here as the type might have - // already been cleared. This is also why we must check if ht_module != NULL. - // Subclasses also cannot make use of the free list. + // already been cleared, which is also why we must check if ht_module != NULL. + // Due to this restriction, subclasses that belong to a different module + // will not be able to use the free list. if (module && _PyModule_GetDef(module) == &_asynciomodule) { state = get_asyncio_state(module); } From 67f8c59fb3f39a3a276ec71ee1c75cc3f9999921 Mon Sep 17 00:00:00 2001 From: Savannah Ostrowski Date: Thu, 18 Apr 2024 02:42:17 +0000 Subject: [PATCH 11/11] Add comment to assertion --- Modules/_asynciomodule.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index ad95966800613a..0873d32a9ec1a5 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -1602,6 +1602,7 @@ FutureIter_dealloc(futureiterobject *it) { PyTypeObject *tp = Py_TYPE(it); + // FutureIter is a heap type so any subclass must also be a heap type. assert(_PyType_HasFeature(tp, Py_TPFLAGS_HEAPTYPE)); PyObject *module = ((PyHeapTypeObject*)tp)->ht_module;