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.
78 changes: 52 additions & 26 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,61 @@ 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))
{
(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_dealloc(pysqlite_Connection *self)
connection_finalize(PyObject *self)
{
PyTypeObject *tp = Py_TYPE(self);
PyObject_GC_UnTrack(self);
tp->tp_clear((PyObject *)self);
pysqlite_Connection *con = (pysqlite_Connection *)self;
PyObject *exc = PyErr_GetRaisedException();

/*
* Before implicitly closing, make sure we're not accidentally part
* of the interpreter tear-down; in that case, we must not call back
* into Python code.
*/
if (_Py_IsInterpreterFinalizing(PyInterpreterState_Get())) {
remove_callbacks(con->db);
}

/* Clean up if user has not called .close() explicitly. */
connection_close(self);
if (con->db) {
if (PyErr_ResourceWarning(self, 1, "unclosed database in %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(PyObject *self)
{
if (PyObject_CallFinalizerFromDealloc(self) < 0) {
return;
}
PyTypeObject *tp = Py_TYPE(self);
PyObject_GC_UnTrack(self);
tp->tp_clear(self);
tp->tp_free(self);
Py_DECREF(tp);
}
Expand Down Expand Up @@ -621,7 +644,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 +2580,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