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 00000000000000..ff499ced73a309 --- /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 0819acd0940964..282855f8865940 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},