[Python-checkins] gh-108083: Don't ignore exceptions in sqlite3.Connection.__init__() and .close() (#108084)
erlend-aasland
webhook-mailer at python.org
Fri Aug 18 07:39:16 EDT 2023
https://github.com/python/cpython/commit/fd195092204aa7fc9f13c5c6d423bc723d0b3520
commit: fd195092204aa7fc9f13c5c6d423bc723d0b3520
branch: main
author: Erlend E. Aasland <erlend at python.org>
committer: erlend-aasland <erlend.aasland at protonmail.com>
date: 2023-08-18T11:39:12Z
summary:
gh-108083: Don't ignore exceptions in sqlite3.Connection.__init__() and .close() (#108084)
- Add explanatory comments
- Add return value to connection_close() for propagating errors
- Always check the return value of connection_exec_stmt()
- Assert pre/post state in remove_callbacks()
- Don't log unraisable exceptions in case of interpreter shutdown
- Make sure we're not initialized if reinit fails
- Try to close the database even if ROLLBACK fails
Co-authored-by: Victor Stinner <vstinner at python.org>
Co-authored-by: Serhiy Storchaka <storchaka at gmail.com>
files:
A Misc/NEWS.d/next/Library/2023-08-17-12-59-35.gh-issue-108083.9J7UcT.rst
M Modules/_sqlite/connection.c
diff --git a/Misc/NEWS.d/next/Library/2023-08-17-12-59-35.gh-issue-108083.9J7UcT.rst b/Misc/NEWS.d/next/Library/2023-08-17-12-59-35.gh-issue-108083.9J7UcT.rst
new file mode 100644
index 0000000000000..ff499ced73a30
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2023-08-17-12-59-35.gh-issue-108083.9J7UcT.rst
@@ -0,0 +1,3 @@
+Fix bugs in the constructor of :mod:`sqlite3.Connection` and
+:meth:`sqlite3.Connection.close` where exceptions could be leaked. Patch by
+Erlend E. Aasland.
diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c
index 0819acd094096..282855f886594 100644
--- a/Modules/_sqlite/connection.c
+++ b/Modules/_sqlite/connection.c
@@ -149,7 +149,7 @@ static void _pysqlite_drop_unused_cursor_references(pysqlite_Connection* self);
static void free_callback_context(callback_context *ctx);
static void set_callback_context(callback_context **ctx_pp,
callback_context *ctx);
-static void connection_close(pysqlite_Connection *self);
+static int connection_close(pysqlite_Connection *self);
PyObject *_pysqlite_query_execute(pysqlite_Cursor *, int, PyObject *, PyObject *);
static PyObject *
@@ -247,10 +247,13 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, PyObject *database,
}
if (self->initialized) {
+ self->initialized = 0;
+
PyTypeObject *tp = Py_TYPE(self);
tp->tp_clear((PyObject *)self);
- connection_close(self);
- self->initialized = 0;
+ if (connection_close(self) < 0) {
+ return -1;
+ }
}
// Create and configure SQLite database object.
@@ -334,7 +337,9 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, PyObject *database,
self->initialized = 1;
if (autocommit == AUTOCOMMIT_DISABLED) {
- (void)connection_exec_stmt(self, "BEGIN");
+ if (connection_exec_stmt(self, "BEGIN") < 0) {
+ return -1;
+ }
}
return 0;
@@ -432,48 +437,83 @@ free_callback_contexts(pysqlite_Connection *self)
static void
remove_callbacks(sqlite3 *db)
{
- sqlite3_trace_v2(db, SQLITE_TRACE_STMT, 0, 0);
+ /* None of these APIs can fail, as long as they are given a valid
+ * database pointer. */
+ assert(db != NULL);
+ int rc = sqlite3_trace_v2(db, SQLITE_TRACE_STMT, 0, 0);
+ assert(rc == SQLITE_OK), (void)rc;
+
sqlite3_progress_handler(db, 0, 0, (void *)0);
- (void)sqlite3_set_authorizer(db, NULL, NULL);
+
+ rc = sqlite3_set_authorizer(db, NULL, NULL);
+ assert(rc == SQLITE_OK), (void)rc;
}
-static void
+static int
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");
+ if (self->db == NULL) {
+ return 0;
+ }
+
+ int rc = 0;
+ if (self->autocommit == AUTOCOMMIT_DISABLED &&
+ !sqlite3_get_autocommit(self->db))
+ {
+ if (connection_exec_stmt(self, "ROLLBACK") < 0) {
+ rc = -1;
}
+ }
- free_callback_contexts(self);
+ sqlite3 *db = self->db;
+ self->db = NULL;
- sqlite3 *db = self->db;
- self->db = NULL;
+ Py_BEGIN_ALLOW_THREADS
+ /* The v2 close call always returns SQLITE_OK if given a valid database
+ * pointer (which we do), so we can safely ignore the return value */
+ (void)sqlite3_close_v2(db);
+ Py_END_ALLOW_THREADS
- Py_BEGIN_ALLOW_THREADS
- int rc = sqlite3_close_v2(db);
- assert(rc == SQLITE_OK), (void)rc;
- Py_END_ALLOW_THREADS
- }
+ free_callback_contexts(self);
+ return rc;
}
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();
+
+ /* If close is implicitly called as a result of interpreter
+ * tear-down, we must not call back into Python. */
+ PyInterpreterState *interp = PyInterpreterState_Get();
+ int teardown = _Py_IsInterpreterFinalizing(interp);
+ if (teardown && con->db) {
+ remove_callbacks(con->db);
+ }
/* Clean up if user has not called .close() explicitly. */
- connection_close(self);
+ if (connection_close(con) < 0) {
+ if (teardown) {
+ PyErr_Clear();
+ }
+ else {
+ PyErr_WriteUnraisable((PyObject *)self);
+ }
+ }
+
+ 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);
}
@@ -621,7 +661,9 @@ pysqlite_connection_close_impl(pysqlite_Connection *self)
pysqlite_close_all_blobs(self);
Py_CLEAR(self->statement_cache);
- connection_close(self);
+ if (connection_close(self) < 0) {
+ return NULL;
+ }
Py_RETURN_NONE;
}
@@ -2555,6 +2597,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},
More information about the Python-checkins
mailing list