From fa106a685c1f199aca5be5c2d0277a14cc9057bd Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Sat, 5 Jun 2021 03:50:39 +0100 Subject: [PATCH] bpo-44304: Fix crash in the sqlite3 module when the GC clears Statement objects (GH-26545) --- Lib/sqlite3/test/userfunctions.py | 17 +++++++++++++++++ .../2021-06-05-02-34-57.bpo-44304._MAoPc.rst | 2 ++ Modules/_sqlite/statement.c | 11 ++++------- 3 files changed, 23 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2021-06-05-02-34-57.bpo-44304._MAoPc.rst diff --git a/Lib/sqlite3/test/userfunctions.py b/Lib/sqlite3/test/userfunctions.py index 6f57d1911b2..42908907249 100644 --- a/Lib/sqlite3/test/userfunctions.py +++ b/Lib/sqlite3/test/userfunctions.py @@ -23,6 +23,7 @@ import unittest import unittest.mock +import gc import sqlite3 as sqlite def func_returntext(): @@ -322,6 +323,22 @@ class FunctionTests(unittest.TestCase): with self.assertRaises(TypeError): self.con.create_function("deterministic", 0, int, True) + def test_function_destructor_via_gc(self): + # See bpo-44304: The destructor of the user function can + # crash if is called without the GIL from the gc functions + dest = sqlite.connect(':memory:') + def md5sum(t): + return + + dest.create_function("md5", 1, md5sum) + x = dest("create table lang (name, first_appeared)") + del md5sum, dest + + y = [x] + y.append(y) + + del x,y + gc.collect() class AggregateTests(unittest.TestCase): def setUp(self): diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-06-05-02-34-57.bpo-44304._MAoPc.rst b/Misc/NEWS.d/next/Core and Builtins/2021-06-05-02-34-57.bpo-44304._MAoPc.rst new file mode 100644 index 00000000000..89104e8e387 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2021-06-05-02-34-57.bpo-44304._MAoPc.rst @@ -0,0 +1,2 @@ +Fix a crash in the :mod:`sqlite3` module that happened when the garbage +collector clears :class:`sqlite.Statement` objects. Patch by Pablo Galindo diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index c86645ad42b..c9dd8829453 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -403,6 +403,10 @@ stmt_dealloc(pysqlite_Statement *self) if (self->in_weakreflist != NULL) { PyObject_ClearWeakRefs((PyObject*)self); } + if (self->st) { + sqlite3_finalize(self->st); + self->st = 0; + } tp->tp_clear((PyObject *)self); tp->tp_free(self); Py_DECREF(tp); @@ -411,13 +415,6 @@ stmt_dealloc(pysqlite_Statement *self) static int stmt_clear(pysqlite_Statement *self) { - if (self->st) { - Py_BEGIN_ALLOW_THREADS - sqlite3_finalize(self->st); - Py_END_ALLOW_THREADS - self->st = 0; - } - Py_CLEAR(self->sql); return 0; }