[Python-checkins] bpo-42213: Remove redundant cyclic GC hack in sqlite3 (GH-26517)
miss-islington
webhook-mailer at python.org
Thu Jun 3 12:38:29 EDT 2021
https://github.com/python/cpython/commit/d88b47b5a396aa8d66f9a0e6b13a0825d59d0eff
commit: d88b47b5a396aa8d66f9a0e6b13a0825d59d0eff
branch: main
author: Erlend Egeberg Aasland <erlend.aasland at innova.no>
committer: miss-islington <31488909+miss-islington at users.noreply.github.com>
date: 2021-06-03T09:38:19-07:00
summary:
bpo-42213: Remove redundant cyclic GC hack in sqlite3 (GH-26517)
The sqlite3 module now fully implements the GC protocol, so there's no
need for this workaround anymore.
- Add and use managed resource helper for connections using TESTFN
- Sort test imports
- Split open-tests into their own test case
Automerge-Triggered-By: GH:vstinner
files:
M Lib/sqlite3/test/dbapi.py
M Lib/sqlite3/test/hooks.py
M Modules/_sqlite/cache.c
M Modules/_sqlite/cache.h
M Modules/_sqlite/connection.c
diff --git a/Lib/sqlite3/test/dbapi.py b/Lib/sqlite3/test/dbapi.py
index ab3313533940a..77fafe093002e 100644
--- a/Lib/sqlite3/test/dbapi.py
+++ b/Lib/sqlite3/test/dbapi.py
@@ -20,14 +20,26 @@
# misrepresented as being the original software.
# 3. This notice may not be removed or altered from any source distribution.
-import threading
-import unittest
+import contextlib
import sqlite3 as sqlite
import sys
+import threading
+import unittest
from test.support.os_helper import TESTFN, unlink
+# Helper for tests using TESTFN
+ at contextlib.contextmanager
+def managed_connect(*args, **kwargs):
+ cx = sqlite.connect(*args, **kwargs)
+ try:
+ yield cx
+ finally:
+ cx.close()
+ unlink(TESTFN)
+
+
class ModuleTests(unittest.TestCase):
def test_api_level(self):
self.assertEqual(sqlite.apilevel, "2.0",
@@ -190,26 +202,27 @@ def test_in_transaction_ro(self):
with self.assertRaises(AttributeError):
self.cx.in_transaction = True
+class OpenTests(unittest.TestCase):
+ _sql = "create table test(id integer)"
+
def test_open_with_path_like_object(self):
""" Checks that we can successfully connect to a database using an object that
is PathLike, i.e. has __fspath__(). """
- self.addCleanup(unlink, TESTFN)
class Path:
def __fspath__(self):
return TESTFN
path = Path()
- with sqlite.connect(path) as cx:
- cx.execute('create table test(id integer)')
+ with managed_connect(path) as cx:
+ cx.execute(self._sql)
def test_open_uri(self):
- self.addCleanup(unlink, TESTFN)
- with sqlite.connect(TESTFN) as cx:
- cx.execute('create table test(id integer)')
- with sqlite.connect('file:' + TESTFN, uri=True) as cx:
- cx.execute('insert into test(id) values(0)')
- with sqlite.connect('file:' + TESTFN + '?mode=ro', uri=True) as cx:
- with self.assertRaises(sqlite.OperationalError):
- cx.execute('insert into test(id) values(1)')
+ with managed_connect(TESTFN) as cx:
+ cx.execute(self._sql)
+ with managed_connect(f"file:{TESTFN}", uri=True) as cx:
+ cx.execute(self._sql)
+ with self.assertRaises(sqlite.OperationalError):
+ with managed_connect(f"file:{TESTFN}?mode=ro", uri=True) as cx:
+ cx.execute(self._sql)
class CursorTests(unittest.TestCase):
diff --git a/Lib/sqlite3/test/hooks.py b/Lib/sqlite3/test/hooks.py
index 8c60bdcf5d70a..520a5b9f11cd4 100644
--- a/Lib/sqlite3/test/hooks.py
+++ b/Lib/sqlite3/test/hooks.py
@@ -254,11 +254,15 @@ def trace(statement):
self.addCleanup(unlink, TESTFN)
con1 = sqlite.connect(TESTFN, isolation_level=None)
con2 = sqlite.connect(TESTFN)
- con1.set_trace_callback(trace)
- cur = con1.cursor()
- cur.execute(queries[0])
- con2.execute("create table bar(x)")
- cur.execute(queries[1])
+ try:
+ con1.set_trace_callback(trace)
+ cur = con1.cursor()
+ cur.execute(queries[0])
+ con2.execute("create table bar(x)")
+ cur.execute(queries[1])
+ finally:
+ con1.close()
+ con2.close()
self.assertEqual(traced_statements, queries)
diff --git a/Modules/_sqlite/cache.c b/Modules/_sqlite/cache.c
index fd4e619f6a011..8196e3c578372 100644
--- a/Modules/_sqlite/cache.c
+++ b/Modules/_sqlite/cache.c
@@ -97,9 +97,6 @@ pysqlite_cache_init(pysqlite_Cache *self, PyObject *args, PyObject *kwargs)
}
self->factory = Py_NewRef(factory);
-
- self->decref_factory = 1;
-
return 0;
}
@@ -108,9 +105,7 @@ cache_traverse(pysqlite_Cache *self, visitproc visit, void *arg)
{
Py_VISIT(Py_TYPE(self));
Py_VISIT(self->mapping);
- if (self->decref_factory) {
- Py_VISIT(self->factory);
- }
+ Py_VISIT(self->factory);
pysqlite_Node *node = self->first;
while (node) {
@@ -124,9 +119,7 @@ static int
cache_clear(pysqlite_Cache *self)
{
Py_CLEAR(self->mapping);
- if (self->decref_factory) {
- Py_CLEAR(self->factory);
- }
+ Py_CLEAR(self->factory);
/* iterate over all nodes and deallocate them */
pysqlite_Node *node = self->first;
diff --git a/Modules/_sqlite/cache.h b/Modules/_sqlite/cache.h
index 083356f93f9e4..209c80dcd54ad 100644
--- a/Modules/_sqlite/cache.h
+++ b/Modules/_sqlite/cache.h
@@ -52,10 +52,6 @@ typedef struct
pysqlite_Node* first;
pysqlite_Node* last;
-
- /* if set, decrement the factory function when the Cache is deallocated.
- * this is almost always desirable, but not in the pysqlite context */
- int decref_factory;
} pysqlite_Cache;
extern PyTypeObject *pysqlite_NodeType;
diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c
index 62c4dc3bbb394..c1a5677d490ef 100644
--- a/Modules/_sqlite/connection.c
+++ b/Modules/_sqlite/connection.c
@@ -149,14 +149,6 @@ pysqlite_connection_init(pysqlite_Connection *self, PyObject *args,
return -1;
}
- /* By default, the Cache class INCREFs the factory in its initializer, and
- * decrefs it in its deallocator method. Since this would create a circular
- * reference here, we're breaking it by decrementing self, and telling the
- * cache class to not decref the factory (self) in its deallocator.
- */
- self->statement_cache->decref_factory = 0;
- Py_DECREF(self);
-
self->detect_types = detect_types;
self->timeout = timeout;
(void)sqlite3_busy_timeout(self->db, (int)(timeout*1000));
More information about the Python-checkins
mailing list