https://github.com/python/cpython/commit/9b648a95ccb4c3b14f1e87158f5c9f5dbb2... commit: 9b648a95ccb4c3b14f1e87158f5c9f5dbb2f62c0 branch: master author: Pablo Galindo <Pablogsal@gmail.com> committer: GitHub <noreply@github.com> date: 2020-09-01T19:39:46+01:00 summary: bpo-41654: Fix deallocator of MemoryError to account for subclasses (GH-22020) When allocating MemoryError classes, there is some logic to use pre-allocated instances in a freelist only if the type that is being allocated is not a subclass of MemoryError. Unfortunately in the destructor this logic is not present so the freelist is altered even with subclasses of MemoryError. files: A Misc/NEWS.d/next/Core and Builtins/2020-08-30-20-38-33.bpo-41654.HtnhAM.rst M Lib/test/test_exceptions.py M Objects/exceptions.c diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index 2ffe8caa03f81..1ec446887770e 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -1,6 +1,7 @@ # Python test set -- part 5, built-in exceptions import copy +import gc import os import sys import unittest @@ -1330,6 +1331,36 @@ def test_assert_shadowing(self): del AssertionError self.fail('Expected exception') + def test_memory_error_subclasses(self): + # bpo-41654: MemoryError instances use a freelist of objects that are + # linked using the 'dict' attribute when they are inactive/dead. + # Subclasses of MemoryError should not participate in the freelist + # schema. This test creates a MemoryError object and keeps it alive + # (therefore advancing the freelist) and then it creates and destroys a + # subclass object. Finally, it checks that creating a new MemoryError + # succeeds, proving that the freelist is not corrupted. + + class TestException(MemoryError): + pass + + try: + raise MemoryError + except MemoryError as exc: + inst = exc + + try: + raise TestException + except Exception: + pass + + for _ in range(10): + try: + raise MemoryError + except MemoryError as exc: + pass + + gc_collect() + class ImportErrorTests(unittest.TestCase): diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-08-30-20-38-33.bpo-41654.HtnhAM.rst b/Misc/NEWS.d/next/Core and Builtins/2020-08-30-20-38-33.bpo-41654.HtnhAM.rst new file mode 100644 index 0000000000000..e05c3133e1262 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2020-08-30-20-38-33.bpo-41654.HtnhAM.rst @@ -0,0 +1,2 @@ +Fix a crash that occurred when destroying subclasses of +:class:`MemoryError`. Patch by Pablo Galindo. diff --git a/Objects/exceptions.c b/Objects/exceptions.c index 1195ba17922dd..b08cbdd6aed35 100644 --- a/Objects/exceptions.c +++ b/Objects/exceptions.c @@ -2286,8 +2286,11 @@ MemoryError_new(PyTypeObject *type, PyObject *args, PyObject *kwds) { PyBaseExceptionObject *self; - if (type != (PyTypeObject *) PyExc_MemoryError) + /* If this is a subclass of MemoryError, don't use the freelist + * and just return a fresh object */ + if (type != (PyTypeObject *) PyExc_MemoryError) { return BaseException_new(type, args, kwds); + } struct _Py_exc_state *state = get_exc_state(); if (state->memerrors_freelist == NULL) { @@ -2313,9 +2316,16 @@ MemoryError_new(PyTypeObject *type, PyObject *args, PyObject *kwds) static void MemoryError_dealloc(PyBaseExceptionObject *self) { - _PyObject_GC_UNTRACK(self); BaseException_clear(self); + /* If this is a subclass of MemoryError, we don't need to + * do anything in the free-list*/ + if (!Py_IS_TYPE(self, (PyTypeObject *) PyExc_MemoryError)) { + return Py_TYPE(self)->tp_free((PyObject *)self); + } + + _PyObject_GC_UNTRACK(self); + struct _Py_exc_state *state = get_exc_state(); if (state->memerrors_numfree >= MEMERRORS_SAVE) { Py_TYPE(self)->tp_free((PyObject *)self);