8000 bpo-42213: Remove redundant cyclic GC hack in sqlite3 by erlend-aasland · Pull Request #26517 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

8000 bpo-42213: Remove redundant cyclic GC hack in sqlite3 #26517

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 26 additions & 13 deletions Lib/sqlite3/test/dbapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
@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",
Expand Down Expand Up @@ -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):
Expand Down
14 changes: 9 additions & 5 deletions Lib/sqlite3/test/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down
11 changes: 2 additions & 9 deletions Modules/_sqlite/cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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) {
Expand All @@ -124,9 +119,7 @@ static int
cache_clear(pysqlite_Cache *self)
{
Py_CLEAR(self->mapping);
if (self->decref_factory) {
8000 Py_CLEAR(self->factory);
}
Py_CLEAR(self->factory);

/* iterate over all nodes and deallocate them */
pysqlite_Node *node = self->first;
Expand Down
4 changes: 0 additions & 4 deletions Modules/_sqlite/cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 0 additions & 8 deletions Modules/_sqlite/connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
0