8000 bpo-42213: Remove redundant cyclic GC hack in sqlite3 (GH-26517) · python/cpython@d88b47b · GitHub
[go: up one dir, main page]
Skip to content

Commit d88b47b

Browse files
author
Erlend Egeberg Aasland
authored
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
1 parent 2c1e258 commit d88b47b

File tree

5 files changed

+37
-39
lines changed

5 files changed

+37
-39
lines changed

‎Lib/sqlite3/test/dbapi.py

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,26 @@
2020
# misrepresented as being the original software.
2121
# 3. This notice may not be removed or altered from any source distribution.
2222

23-
import threading
24-
import unittest
23+
import contextlib
2524
import sqlite3 as sqlite
2625
import sys
26+
import threading
27+
import unittest
2728

2829
from test.support.os_helper import TESTFN, unlink
2930

3031

32+
# Helper for tests using TESTFN
33+
@contextlib.contextmanager
34+
def managed_connect(*args, **kwargs):
35+
cx = sqlite.connect(*args, **kwargs)
36+
try:
37+
yield cx
38+
finally:
39+
cx.close()
40+
unlink(TESTFN)
41+
42+
3143
class ModuleTests(unittest.TestCase):
3244
def test_api_level(self):
3345
self.assertEqual(sqlite.apilevel, "2.0",
@@ -190,26 +202,27 @@ def test_in_transaction_ro(self):
190202
with self.assertRaises(AttributeError):
191203
self.cx.in_transaction = True
192204

205+
class OpenTests(unittest.TestCase):
206+
_sql = "create table test(id integer)"
207+
193208
def test_open_with_path_like_object(self):
194209
""" Checks that we can successfully connect to a database using an object that
195210
is PathLike, i.e. has __fspath__(). """
196-
self.addCleanup(unlink, TESTFN)
197211
class Path:
198212
def __fspath__(self):
199213
return TESTFN
200214
path = Path()
201-
with sqlite.connect(path) as cx:
202-
cx.execute('create table test(id integer)')
215+
with managed_connect(path) as cx:
216+
cx.execute(self._sql)
203217

204218
def test_open_uri(self):
205-
self.addCleanup(unlink, TESTFN)
206-
with sqlite.connect(TESTFN) as cx:
207-
cx.execute('create table test(id integer)')
208-
with sqlite.connect('file:' + TESTFN, uri=True) as cx:
209-
cx.execute('insert into test(id) values(0)')
210-
with sqlite.connect('file:' + TESTFN + '?mode=ro', uri=True) as cx:
211-
with self.assertRaises(sqlite.OperationalError):
212-
cx.execute('insert into test(id) values(1)')
219+
with managed_connect(TESTFN) as cx:
220+
cx.execute(self._sql)
221+
with managed_connect(f"file:{TESTFN}", uri=True) as cx:
222+
cx.execute(self._sql)
223+
with self.assertRaises(sqlite.OperationalError):
224+
with managed_connect(f"file:{TESTFN}?mode=ro", uri=True) as cx:
225+
cx.execute(self._sql)
213226

214227

215228
class CursorTests(unittest.TestCase):

‎Lib/sqlite3/test/hooks.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -254,11 +254,15 @@ def trace(statement):
254254
self.addCleanup(unlink, TESTFN)
255255
con1 = sqlite.connect(TESTFN, isolation_level=None)
256256
con2 = sqlite.connect(TESTFN)
257-
con1.set_trace_callback(trace)
258-
cur = con1.cursor()
259-
cur.execute(queries[0])
260-
con2.execute("create table bar(x)")
261-
cur.execute(queries[1])
257+
try:
258+
con1.set_trace_callback(trace)
259+
cur = con1.cursor()
260+
cur.execute(queries[0])
261+
con2.execute("create table bar(x)")
262+
cur.execute(queries[1])
263+
finally:
264+
con1.close()
265+
con2.close()
262266
self.assertEqual(traced_statements, queries)
263267

264268

‎Modules/_sqlite/cache.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,6 @@ pysqlite_cache_init(pysqlite_Cache *self, PyObject *args, PyObject *kwargs)
9797
}
9898

9999
self->factory = Py_NewRef(factory);
100-
101-
self->decref_factory = 1;
102-
103100
return 0;
104101
}
105102

@@ -108,9 +105,7 @@ cache_traverse(pysqlite_Cache *self, visitproc visit, void *arg)
108105
{
109106
Py_VISIT(Py_TYPE(self));
110107
Py_VISIT(self->mapping);
111-
if (self->decref_factory) {
112-
Py_VISIT(self->factory);
113-
}
108+
Py_VISIT(self->factory);
114109

115110
pysqlite_Node *node = self->first;
116111
while (node) {
@@ -124,9 +119,7 @@ static int
124119
cache_clear(pysqlite_Cache *self)
125120
{
126121
Py_CLEAR(self->mapping);
127-
if (self->decref_factory) {
128-
Py_CLEAR(self->factory);
129-
}
122+
Py_CLEAR(self->factory);
130123

131124
/* iterate over all nodes and deallocate them */
132125
pysqlite_Node *node = self->first;

‎Modules/_sqlite/cache.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,6 @@ typedef struct
5252

5353
pysqlite_Node* first;
5454
pysqlite_Node* last;
55-
56-
/* if set, decrement the factory function when the Cache is deallocated.
57-
* this is almost always desirable, but not in the pysqlite context */
58-
int decref_factory;
5955
} pysqlite_Cache;
6056

6157
extern PyTypeObject *pysqlite_NodeType;

‎Modules/_sqlite/connection.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -149,14 +149,6 @@ pysqlite_connection_init(pysqlite_Connection *self, PyObject *args,
149149
return -1;
150150
}
151151

152-
/* By default, the Cache class INCREFs the factory in its initializer, and
153-
* decrefs it in its deallocator method. Since this would create a circular
154-
* reference here, we're breaking it by decrementing self, and telling the
155-
* cache class to not decref the factory (self) in its deallocator.
156-
*/
157-
self->statement_cache->decref_factory = 0;
158-
Py_DECREF(self);
159-
160152
self->detect_types = detect_types;
161153
self->timeout = timeout;
162154
(void)sqlite3_busy_timeout(self->db, (int)(timeout*1000));

0 commit comments

Comments
 (0)
0