8000 [3.12] gh-108083: Don't ignore exceptions in sqlite3.Connection.__ini… · python/cpython@0c21298 · GitHub
[go: up one dir, main page]
Skip to content

Commit 0c21298

Browse files
erlend-aaslandvstinnerserhiy-storchaka
authored
[3.12] gh-108083: Don't ignore exceptions in sqlite3.Connection.__init__() and .close() (#108084) (#108141)
- 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 (cherry picked from commit fd19509) Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
1 parent 41634ed commit 0c21298

File tree

2 files changed

+78
-31
lines changed

2 files changed

+78
-31
lines changed

‎Misc/NEWS.d/next/Library/2023-08-17-12-59-35.gh-issue-108083.9J7UcT.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix bugs in the constructor of :mod:`sqlite3.Connection` and
2+
:meth:`sqlite3.Connection.close` where exceptions could be leaked. Patch by
3+
Erlend E. Aasland.

‎Modules/_sqlite/connection.c

Lines changed: 75 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ static void _pysqlite_drop_unused_cursor_references(pysqlite_Connection* self);
146146
static void free_callback_context(callback_context *ctx);
147147
static void set_callback_context(callback_context **ctx_pp,
148148
callback_context *ctx);
149-
static void connection_close(pysqlite_Connection *self);
149+
static int connection_close(pysqlite_Connection *self);
150150
PyObject *_pysqlite_query_execute(pysqlite_Cursor *, int, PyObject *, PyObject *);
151151

152152
static PyObject *
@@ -244,10 +244,13 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, PyObject *database,
244244
}
245245

246246
if (self->initialized) {
247+
self->initialized = 0;
248+
247249
PyTypeObject *tp = Py_TYPE(self);
248250
tp->tp_clear((PyObject *)self);
249-
connection_close(self);
250-
self->initialized = 0;
251+
if (connection_close(self) < 0) {
252+
return -1;
253+
}
251254
}
252255

253256
// Create and configure SQLite database object.
@@ -331,7 +334,9 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, PyObject *database,
331334
self->initialized = 1;
332335

333336
if (autocommit == AUTOCOMMIT_DISABLED) {
334-
(void)connection_exec_stmt(self, "BEGIN");
337+
if (connection_exec_stmt(self, "BEGIN") < 0) {
338+
return -1;
339+
}
335340
}
336341
return 0;
337342

@@ -401,52 +406,88 @@ free_callback_contexts(pysqlite_Connection *self)
401406
static void
402407
remove_callbacks(sqlite3 *db)
403408
{
409+
assert(db != NULL);
410+
/* None of these APIs can fail, as long as they are given a valid
411+
* database pointer. */
412+
int rc;
404413
#ifdef HAVE_TRACE_V2
405-
sqlite3_trace_v2(db, SQLITE_TRACE_STMT, 0, 0);
414+
rc = sqlite3_trace_v2(db, SQLITE_TRACE_STMT, 0, 0);
415+
assert(rc == SQLITE_OK), (void)rc;
406416
#else
407417
sqlite3_trace(db, 0, (void*)0);
408418
#endif
419+
409420
sqlite3_progress_handler(db, 0, 0, (void *)0);
410-
(void)sqlite3_set_authorizer(db, NULL, NULL);
421+
422+
rc = sqlite3_set_authorizer(db, NULL, NULL);
423+
assert(rc == SQLITE_OK), (void)rc;
411424
}
412425

413-
static void
426+
static int
414427
connection_close(pysqlite_Connection *self)
415428
{
416-
if (self->db) {
417-
if (self->autocommit == AUTOCOMMIT_DISABLED &&
418-
!sqlite3_get_autocommit(self->db))
419-
{
420-
/* If close is implicitly called as a result of interpreter
421-
* tear-down, we must not call back into Python. */
422-
if (_Py_IsInterpreterFinalizing(PyInterpreterState_Get())) {
423-
remove_callbacks(self->db);
424-
}
425-
(void)connection_exec_stmt(self, "ROLLBACK");
429+
if (self->db == NULL) {
430+
return 0;
431+
}
432+
433+
int rc = 0;
434+
if (self->autocommit == AUTOCOMMIT_DISABLED &&
435+
!sqlite3_get_autocommit(self->db))
436+
{
437+
if (connection_exec_stmt(self, "ROLLBACK") < 0) {
438+
rc = -1;
426439
}
440+
}
427441

428-
free_callback_contexts(self);
442+
sqlite3 *db = self->db;
443+
self->db = NULL;
429444

430-
sqlite3 *db = self->db;
431-
self->db = NULL;
445+
Py_BEGIN_ALLOW_THREADS
446+
/* The v2 close call always returns SQLITE_OK if given a valid database
447+
* pointer (which we do), so we can safely ignore the return value */
448+
(void)sqlite3_close_v2(db);
449+
Py_END_ALLOW_THREADS
432450

433-
Py_BEGIN_ALLOW_THREADS
434-
int rc = sqlite3_close_v2(db);
435-
assert(rc == SQLITE_OK), (void)rc;
436-
Py_END_ALLOW_THREADS
437-
}
451+
free_callback_contexts(self);
452+
return rc;
438453
}
439454

440455
static void
441-
connection_dealloc(pysqlite_Connection *self)
456+
connection_finalize(PyObject *self)
442457
{
443-
PyTypeObject *tp = Py_TYPE(self);
444-
PyObject_GC_UnTrack(self);
445-
tp->tp_clear((PyObject *)self);
458+
pysqlite_Connection *con = (pysqlite_Connection *)self;
459+
PyObject *exc = PyErr_GetRaisedException();
460+
461+
/* If close is implicitly called as a result of interpreter
462+
* tear-down, we must not call back into Python. */
463+
PyInterpreterState *interp = PyInterpreterState_Get();
464+
int teardown = _Py_IsInterpreterFinalizing(interp);
465+
if (teardown && con->db) {
466+
remove_callbacks(con->db);
467+
}
446468

447469
/* Clean up if user has not called .close() explicitly. */
448-
connection_close(self);
470+
if (connection_close(con) < 0) {
471+
if (teardown) {
472+
PyErr_Clear();
473+
}
474+
else {
475+
PyErr_WriteUnraisable((PyObject *)self);
476+
}
477+
}
478+
479+
PyErr_SetRaisedException(exc);
480+
}
449481

482+
static void
483+
connection_dealloc(PyObject *self)
484+
{
485+
if (PyObject_CallFinalizerFromDealloc(self) < 0) {
486+
return;
487+
}
488+
PyTypeObject *tp = Py_TYPE(self);
489+
PyObject_GC_UnTrack(self);
490+
tp->tp_clear(self);
450491
tp->tp_free(self);
451492
Py_DECREF(tp);
452493
}
@@ -594,7 +635,9 @@ pysqlite_connection_close_impl(pysqlite_Connection *self)
594635

595636
pysqlite_close_all_blobs(self);
596637
Py_CLEAR(self->statement_cache);
597-
connection_close(self);
638+
if (connection_close(self) < 0) {
639+
return NULL;
640+
}
598641

599642
Py_RETURN_NONE;
600643
}
@@ -2582,6 +2625,7 @@ static struct PyMemberDef connection_members[] =
25822625
};
25832626

25842627
static PyType_Slot connection_slots[] = {
2628+
{Py_tp_finalize, connection_finalize},
25852629
{Py_tp_dealloc, connection_dealloc},
25862630
{Py_tp_doc, (void *)connection_doc},
25872631
{Py_tp_methods, connection_methods},

0 commit comments

Comments
 (0)
0