From a444686c11e7ef768cb006425ed04c53065d6029 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Mon, 9 Aug 2021 14:28:47 +0100 Subject: [PATCH 1/4] bpo-33930: Fix segfault with deep recursion when cleaning method objects --- Lib/test/test_exceptions.py | 10 ++++++++++ .../2021-08-09-14-29-52.bpo-33930.--5LQ-.rst | 2 ++ Objects/methodobject.c | 4 +++- 3 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2021-08-09-14-29-52.bpo-33930.--5LQ-.rst diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index b280cfea435fd4..fad9c07e1264f7 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -1027,6 +1027,16 @@ def g(): self.assertIsInstance(v, RecursionError, type(v)) self.assertIn("maximum recursion depth exceeded", str(v)) + + @cpython_only + def test_crashcan_recursion(self): + def foo(): + o = object() + for x in range(1000000): + o = o.__dir__ + + foo() + @cpython_only def test_recursion_normalizing_exception(self): # Issue #22898. diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-08-09-14-29-52.bpo-33930.--5LQ-.rst b/Misc/NEWS.d/next/Core and Builtins/2021-08-09-14-29-52.bpo-33930.--5LQ-.rst new file mode 100644 index 00000000000000..12829602a521f1 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2021-08-09-14-29-52.bpo-33930.--5LQ-.rst @@ -0,0 +1,2 @@ +Fix segmentation failt with deep recursion when cleaning method objects. +Patch by Augusto Goulart and Pablo Galindo. diff --git a/Objects/methodobject.c b/Objects/methodobject.c index 7b430416c5a048..75c8273ab5b4d8 100644 --- a/Objects/methodobject.c +++ b/Objects/methodobject.c @@ -160,7 +160,8 @@ PyCMethod_GetClass(PyObject *op) static void meth_dealloc(PyCFunctionObject *m) { - _PyObject_GC_UNTRACK(m); + PyObject_GC_UnTrack(m); + Py_TRASHCAN_SAFE_BEGIN(m); if (m->m_weakreflist != NULL) { PyObject_ClearWeakRefs((PyObject*) m); } @@ -170,6 +171,7 @@ meth_dealloc(PyCFunctionObject *m) Py_XDECREF(m->m_self); Py_XDECREF(m->m_module); PyObject_GC_Del(m); + Py_TRASHCAN_SAFE_END(m); } static PyObject * From 6bcd0a02e6acbdf535dcb1e02bfceb09caa64f4d Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Mon, 9 Aug 2021 14:49:56 +0100 Subject: [PATCH 2/4] fixup! bpo-33930: Fix segfault with deep recursion when cleaning method objects --- Lib/test/test_exceptions.py | 1 + Objects/methodobject.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index fad9c07e1264f7..a085da76600eb8 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -1036,6 +1036,7 @@ def foo(): o = o.__dir__ foo() + support.gc_collect() @cpython_only def test_recursion_normalizing_exception(self): diff --git a/Objects/methodobject.c b/Objects/methodobject.c index 75c8273ab5b4d8..98c5607db7db82 100644 --- a/Objects/methodobject.c +++ b/Objects/methodobject.c @@ -161,7 +161,7 @@ static void meth_dealloc(PyCFunctionObject *m) { PyObject_GC_UnTrack(m); - Py_TRASHCAN_SAFE_BEGIN(m); + Py_TRASHCAN_BEGIN(m, meth_dealloc); if (m->m_weakreflist != NULL) { PyObject_ClearWeakRefs((PyObject*) m); } @@ -171,7 +171,7 @@ meth_dealloc(PyCFunctionObject *m) Py_XDECREF(m->m_self); Py_XDECREF(m->m_module); PyObject_GC_Del(m); - Py_TRASHCAN_SAFE_END(m); + Py_TRASHCAN_END; } static PyObject * From 80ac4cf3bacf61ffb4708a92f8f45c3bc51bec68 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Tue, 10 Aug 2021 13:55:24 +0100 Subject: [PATCH 3/4] fixup! fixup! bpo-33930: Fix segfault with deep recursion when cleaning method objects --- Lib/test/test_exceptions.py | 6 +++++- Objects/methodobject.c | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index a085da76600eb8..5acc71202f8f83 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -1030,9 +1030,13 @@ def g(): @cpython_only def test_crashcan_recursion(self): + # See bpo-33930 + def foo(): o = object() - for x in range(1000000): + for x in range(1_000_000): + # Create a big chain of method objects that will trigger + # a deep chain of calls when they need to be destructed. o = o.__dir__ foo() diff --git a/Objects/methodobject.c b/Objects/methodobject.c index 98c5607db7db82..2df63cfdf6a818 100644 --- a/Objects/methodobject.c +++ b/Objects/methodobject.c @@ -160,6 +160,8 @@ PyCMethod_GetClass(PyObject *op) static void meth_dealloc(PyCFunctionObject *m) { + // The Py_TRASHCAN mechanism requires that we be able to + // call PyObject_GC_UnTrack twice on an object. PyObject_GC_UnTrack(m); Py_TRASHCAN_BEGIN(m, meth_dealloc); if (m->m_weakreflist != NULL) { From 738feccc0f380e10e3c763abc7581107a4e4571b Mon Sep 17 00:00:00 2001 From: Pablo Galindo Salgado Date: Tue, 10 Aug 2021 22:26:51 +0100 Subject: [PATCH 4/4] Update Misc/NEWS.d/next/Core and Builtins/2021-08-09-14-29-52.bpo-33930.--5LQ-.rst MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Łukasz Langa --- .../Core and Builtins/2021-08-09-14-29-52.bpo-33930.--5LQ-.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-08-09-14-29-52.bpo-33930.--5LQ-.rst b/Misc/NEWS.d/next/Core and Builtins/2021-08-09-14-29-52.bpo-33930.--5LQ-.rst index 12829602a521f1..827dd3f8b65131 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2021-08-09-14-29-52.bpo-33930.--5LQ-.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2021-08-09-14-29-52.bpo-33930.--5LQ-.rst @@ -1,2 +1,2 @@ -Fix segmentation failt with deep recursion when cleaning method objects. +Fix segmentation fault with deep recursion when cleaning method objects. Patch by Augusto Goulart and Pablo Galindo.