From 50bf715643ee69fcf31f443ce60330f8e32040e2 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Mon, 12 Oct 2020 18:03:40 +0200 Subject: [PATCH 1/3] bpo-42015: Reorder dereferencing calls in meth_dealloc, to make sure m_self is kept alive long enough --- Objects/methodobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/methodobject.c b/Objects/methodobject.c index 5659f2143d1823..79e0f98839101e 100644 --- a/Objects/methodobject.c +++ b/Objects/methodobject.c @@ -164,9 +164,9 @@ meth_dealloc(PyCFunctionObject *m) if (m->m_weakreflist != NULL) { PyObject_ClearWeakRefs((PyObject*) m); } + Py_XDECREF(PyCFunction_GET_CLASS(m)); Py_XDECREF(m->m_self); Py_XDECREF(m->m_module); - Py_XDECREF(PyCFunction_GET_CLASS(m)); PyObject_GC_Del(m); } From 1e69effc06754a6f6887915ac9902cc5840f347a Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Mon, 12 Oct 2020 20:17:24 +0200 Subject: [PATCH 2/3] Add comment on derefering order, change order in meth_traverse as well, and add NEWS blurb --- .../next/C API/2020-10-12-20-13-58.bpo-42015.X4H2_V.rst | 5 +++++ Objects/methodobject.c | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/C API/2020-10-12-20-13-58.bpo-42015.X4H2_V.rst diff --git a/Misc/NEWS.d/next/C API/2020-10-12-20-13-58.bpo-42015.X4H2_V.rst b/Misc/NEWS.d/next/C API/2020-10-12-20-13-58.bpo-42015.X4H2_V.rst new file mode 100644 index 00000000000000..655f59acee7197 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2020-10-12-20-13-58.bpo-42015.X4H2_V.rst @@ -0,0 +1,5 @@ +Order of dereferencing for `PyCFunction` deallocation changed to make sure +the object passed as ``self`` argument is only dereferenced after the last +access to the function's associated `PyMethodDef`. The allows extension +modules to manage the lifetime of dynamically allocated `PyModuleDef` +instances through ``self``. diff --git a/Objects/methodobject.c b/Objects/methodobject.c index 79e0f98839101e..7b430416c5a048 100644 --- a/Objects/methodobject.c +++ b/Objects/methodobject.c @@ -164,6 +164,8 @@ meth_dealloc(PyCFunctionObject *m) if (m->m_weakreflist != NULL) { PyObject_ClearWeakRefs((PyObject*) m); } + // Dereference class before m_self: PyCFunction_GET_CLASS accesses + // PyMethodDef m_ml, which could be kept alive by m_self Py_XDECREF(PyCFunction_GET_CLASS(m)); Py_XDECREF(m->m_self); Py_XDECREF(m->m_module); @@ -243,9 +245,9 @@ meth_get__qualname__(PyCFunctionObject *m, void *closure) static int meth_traverse(PyCFunctionObject *m, visitproc visit, void *arg) { + Py_VISIT(PyCFunction_GET_CLASS(m)); Py_VISIT(m->m_self); Py_VISIT(m->m_module); - Py_VISIT(PyCFunction_GET_CLASS(m)); return 0; } From c508a4f6acd03c81d45626ced0740d35d2b18cf0 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Mon, 12 Oct 2020 21:56:03 +0200 Subject: [PATCH 3/3] Rephrase NEWS fragment to be more user-focussed --- .../next/C API/2020-10-12-20-13-58.bpo-42015.X4H2_V.rst | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Misc/NEWS.d/next/C API/2020-10-12-20-13-58.bpo-42015.X4H2_V.rst b/Misc/NEWS.d/next/C API/2020-10-12-20-13-58.bpo-42015.X4H2_V.rst index 655f59acee7197..d77619f64bb178 100644 --- a/Misc/NEWS.d/next/C API/2020-10-12-20-13-58.bpo-42015.X4H2_V.rst +++ b/Misc/NEWS.d/next/C API/2020-10-12-20-13-58.bpo-42015.X4H2_V.rst @@ -1,5 +1,3 @@ -Order of dereferencing for `PyCFunction` deallocation changed to make sure -the object passed as ``self`` argument is only dereferenced after the last -access to the function's associated `PyMethodDef`. The allows extension -modules to manage the lifetime of dynamically allocated `PyModuleDef` -instances through ``self``. +Fix potential crash in deallocating method objects when dynamically +allocated `PyMethodDef`'s lifetime is managed through the ``self`` +argument of a `PyCFunction`.