Skip to content
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

gh-105539: Emit ResourceWarning if sqlite3 database is not closed explicitly #108015

Merged
merged 12 commits into from
Aug 22, 2023
6 changes: 6 additions & 0 deletions Doc/library/sqlite3.rst
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,12 @@ Connection objects
* :ref:`sqlite3-connection-shortcuts`
* :ref:`sqlite3-connection-context-manager`


.. versionchanged:: 3.13

A :exc:`ResourceWarning` is emitted if :meth:`close` is not called before
a :class:`!Connection` object is deleted.

An SQLite database connection has the following attributes and methods:

.. method:: cursor(factory=Cursor)
Expand Down
7 changes: 7 additions & 0 deletions Doc/whatsnew/3.13.rst
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,13 @@ pathlib
:meth:`~pathlib.Path.is_dir`.
(Contributed by Barney Gale in :gh:`77609` and :gh:`105793`.)

sqlite3
-------

* A :exc:`ResourceWarning` is now emitted if a :class:`sqlite3.Connection`
object is not :meth:`closed <sqlite3.Connection.close>` explicitly.
(Contributed by Erlend E. Aasland in :gh:`105539`.)

traceback
---------

Expand Down
6 changes: 6 additions & 0 deletions Lib/test/test_sqlite3/test_dbapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,12 @@ def test_connect_positional_arguments(self):
cx.close()
self.assertEqual(cm.filename, __file__)

def test_connection_resource_warning(self):
with self.assertWarns(ResourceWarning):
cx = sqlite.connect(":memory:")
del cx
gc_collect()
erlend-aasland marked this conversation as resolved.
Show resolved Hide resolved


class UninitialisedConnectionTests(unittest.TestCase):
def setUp(self):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
:mod:`sqlite3` now emits an :exc:`ResourceWarning` if a
:class:`sqlite3.Connection` object is not :meth:`closed
<sqlite3.connection.close>` explicitly. Patch by Erlend E. Aasland.
71 changes: 46 additions & 25 deletions Modules/_sqlite/connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,9 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, PyObject *database,
if (self->initialized) {
PyTypeObject *tp = Py_TYPE(self);
tp->tp_clear((PyObject *)self);
connection_close(self);
if (self->db) {
connection_close(self);
}
self->initialized = 0;
}

Expand Down Expand Up @@ -440,40 +442,56 @@ remove_callbacks(sqlite3 *db)
static void
connection_close(pysqlite_Connection *self)
{
if (self->db) {
if (self->autocommit == AUTOCOMMIT_DISABLED &&
!sqlite3_get_autocommit(self->db))
{
/* If close is implicitly called as a result of interpreter
* tear-down, we must not call back into Python. */
if (_Py_IsInterpreterFinalizing(PyInterpreterState_Get())) {
remove_callbacks(self->db);
}
(void)connection_exec_stmt(self, "ROLLBACK");
assert(self->db);
if (self->autocommit == AUTOCOMMIT_DISABLED &&
!sqlite3_get_autocommit(self->db))
{
/* If close is implicitly called as a result of interpreter
* tear-down, we must not call back into Python. */
if (_Py_IsInterpreterFinalizing(PyInterpreterState_Get())) {
erlend-aasland marked this conversation as resolved.
Show resolved Hide resolved
remove_callbacks(self->db);
}
(void)connection_exec_stmt(self, "ROLLBACK");
erlend-aasland marked this conversation as resolved.
Show resolved Hide resolved
}

free_callback_contexts(self);
free_callback_contexts(self);

sqlite3 *db = self->db;
self->db = NULL;
sqlite3 *db = self->db;
self->db = NULL;

Py_BEGIN_ALLOW_THREADS
int rc = sqlite3_close_v2(db);
assert(rc == SQLITE_OK), (void)rc;
Py_END_ALLOW_THREADS
Py_BEGIN_ALLOW_THREADS
int rc = sqlite3_close_v2(db);
assert(rc == SQLITE_OK), (void)rc;
Py_END_ALLOW_THREADS
}

static void
connection_finalize(PyObject *self)
{
pysqlite_Connection *con = (pysqlite_Connection *)self;
PyObject *exc = PyErr_GetRaisedException();
/* Clean up if user has not called .close() explicitly. */
if (con->db) {
if (PyErr_ResourceWarning(self, 1, "unclosed %R", self)) {
/* Spurious errors can appear at shutdown */
if (PyErr_ExceptionMatches(PyExc_Warning)) {
erlend-aasland marked this conversation as resolved.
Show resolved Hide resolved
PyErr_WriteUnraisable(self);
erlend-aasland marked this conversation as resolved.
Show resolved Hide resolved
}
}
connection_close(con);
}
PyErr_SetRaisedException(exc);
}

static void
connection_dealloc(pysqlite_Connection *self)
connection_dealloc(PyObject *self)
{
if (PyObject_CallFinalizerFromDealloc(self) < 0) {
return;
}
PyTypeObject *tp = Py_TYPE(self);
PyObject_GC_UnTrack(self);
tp->tp_clear((PyObject *)self);

/* Clean up if user has not called .close() explicitly. */
connection_close(self);

tp->tp_clear(self);
tp->tp_free(self);
Py_DECREF(tp);
}
Expand Down Expand Up @@ -621,7 +639,9 @@ pysqlite_connection_close_impl(pysqlite_Connection *self)

pysqlite_close_all_blobs(self);
Py_CLEAR(self->statement_cache);
connection_close(self);
if (self->db) {
connection_close(self);
}

Py_RETURN_NONE;
}
Expand Down Expand Up @@ -2555,6 +2575,7 @@ static struct PyMemberDef connection_members[] =
};

static PyType_Slot connection_slots[] = {
{Py_tp_finalize, connection_finalize},
{Py_tp_dealloc, connection_dealloc},
{Py_tp_doc, (void *)connection_doc},
{Py_tp_methods, connection_methods},
Expand Down