From cbee5f4e633fee86c2dcccbbd9289b30fccf968b Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Wed, 30 Nov 2022 23:26:40 +0800 Subject: [PATCH 1/5] Fix crash when freeing objects with managed dictionaries --- Lib/test/test_sqlite3/test_regression.py | 12 ++++++++++++ Objects/dictobject.c | 1 + 2 files changed, 13 insertions(+) diff --git a/Lib/test/test_sqlite3/test_regression.py b/Lib/test/test_sqlite3/test_regression.py index 0b727cecb0e8cb..1aae361ac1f7a5 100644 --- a/Lib/test/test_sqlite3/test_regression.py +++ b/Lib/test/test_sqlite3/test_regression.py @@ -469,6 +469,18 @@ def test_executescript_step_through_select(self): con.executescript("select step(t) from t") self.assertEqual(steps, values) + def test_custom_cursor_object_crash_gh_99886(self): + # This test segfaults on GH-99886 + class MyCursor(sqlite3.Cursor): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + # this can go before or after the super call; doesn't matter + self.some_attr = None + + conn = sqlite3.connect(':memory:') + cur = conn.cursor(MyCursor) + cur.close() + del cur class RecursiveUseOfCursors(unittest.TestCase): # GH-80254: sqlite3 should not segfault for recursive use of cursors. diff --git a/Objects/dictobject.c b/Objects/dictobject.c index ebbd22ee7c145e..5452ee274199b5 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -5581,6 +5581,7 @@ _PyObject_FreeInstanceAttributes(PyObject *self) Py_XDECREF((*values_ptr)->values[i]); } free_values(*values_ptr); + *values_ptr = NULL; } PyObject * From c2d92eafa7c691d0f5d0d37c6ca40836aa38179c Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Wed, 30 Nov 2022 15:29:09 +0000 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2022-11-30-15-29-08.gh-issue-99886.feJkSv.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-11-30-15-29-08.gh-issue-99886.feJkSv.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-11-30-15-29-08.gh-issue-99886.feJkSv.rst b/Misc/NEWS.d/next/Core and Builtins/2022-11-30-15-29-08.gh-issue-99886.feJkSv.rst new file mode 100644 index 00000000000000..8bdaa9462750de --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-11-30-15-29-08.gh-issue-99886.feJkSv.rst @@ -0,0 +1 @@ +Fix a crash when an object which does not have a dictionary frees its instance values. From 24e0481f5a3ca9cd342ffc60a6645a633567b5ff Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 1 Dec 2022 00:09:28 +0800 Subject: [PATCH 3/5] fix test --- Lib/test/test_sqlite3/test_regression.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_sqlite3/test_regression.py b/Lib/test/test_sqlite3/test_regression.py index 1aae361ac1f7a5..e23578d13f7080 100644 --- a/Lib/test/test_sqlite3/test_regression.py +++ b/Lib/test/test_sqlite3/test_regression.py @@ -471,13 +471,13 @@ def test_executescript_step_through_select(self): def test_custom_cursor_object_crash_gh_99886(self): # This test segfaults on GH-99886 - class MyCursor(sqlite3.Cursor): + class MyCursor(sqlite.Cursor): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) # this can go before or after the super call; doesn't matter self.some_attr = None - conn = sqlite3.connect(':memory:') + conn = sqlite.connect(':memory:') cur = conn.cursor(MyCursor) cur.close() del cur From ca5f6aa3efa0a2fd8cb32d23bb458ff8493407d7 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 1 Dec 2022 00:11:20 +0800 Subject: [PATCH 4/5] Address code review --- Objects/dictobject.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 5452ee274199b5..4a214f8cf5b751 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -5573,15 +5573,16 @@ _PyObject_FreeInstanceAttributes(PyObject *self) PyTypeObject *tp = Py_TYPE(self); assert(Py_TYPE(self)->tp_flags & Py_TPFLAGS_MANAGED_DICT); PyDictValues **values_ptr = _PyObject_ValuesPointer(self); - if (*values_ptr == NULL) { + PyDictValues *values = *values_ptr; + if (values == NULL) { return; } + *values_ptr = NULL; PyDictKeysObject *keys = CACHED_KEYS(tp); for (Py_ssize_t i = 0; i < keys->dk_nentries; i++) { - Py_XDECREF((*values_ptr)->values[i]); + Py_XDECREF(values->values[i]); } - free_values(*values_ptr); - *values_ptr = NULL; + free_values(values); } PyObject * From 2e0943e9cffff2ce4e98d6b800016cf75723be33 Mon Sep 17 00:00:00 2001 From: Ken Jin Date: Thu, 1 Dec 2022 16:02:33 +0800 Subject: [PATCH 5/5] Apply Erlend's suggestions Co-authored-by: Erlend E. Aasland --- Lib/test/test_sqlite3/test_regression.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_sqlite3/test_regression.py b/Lib/test/test_sqlite3/test_regression.py index e23578d13f7080..9a07e02ae1197e 100644 --- a/Lib/test/test_sqlite3/test_regression.py +++ b/Lib/test/test_sqlite3/test_regression.py @@ -477,10 +477,10 @@ def __init__(self, *args, **kwargs): # this can go before or after the super call; doesn't matter self.some_attr = None - conn = sqlite.connect(':memory:') - cur = conn.cursor(MyCursor) - cur.close() - del cur + with memory_database() as con: + cur = con.cursor(MyCursor) + cur.close() + del cur class RecursiveUseOfCursors(unittest.TestCase): # GH-80254: sqlite3 should not segfault for recursive use of cursors.