8000 gh-108083: Don't ignore exceptions in sqlite3.Connection.__init__() a… · python/cpython@fd19509 · GitHub
[go: up one dir, main page]
Skip to content

Commit fd19509

Browse files
erlend-aaslandvstinnerserhiy-storchaka
authored
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@python.org> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
1 parent 3ff5ef2 commit fd19509

File tree

2 files changed

+77
-31
lines changed

2 files changed

+77
-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: 74 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ static void _pysqlite_drop_unused_cursor_references(pysqlite_Connection* self);
149149
static void free_callback_context(callback_context *ctx);
150150
static void set_callback_context(callback_context **ctx_pp,
151151
callback_context *ctx);
152-
static void connection_close(pysqlite_Connection *self);
152+
static int connection_close(pysqlite_Connection *self);
153153
PyObject *_pysqlite_query_execute(pysqlite_Cursor *, int, PyObject *, PyObject *);
154154

155155
static PyObject *
@@ -247,10 +247,13 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, PyObject *database,
247247
}
248248

249249
if (self->initialized) {
250+
self->initialized = 0;
251+
250252
PyTypeObject *tp = Py_TYPE(self);
251253
tp->tp_clear((PyObject *)self);
252-
connection_close(self);
253-
self->initialized = 0;
254+
if (connection_close(self) < 0) {
255+
return -1;
256+
}
254257
}
255258

256259
// Create and configure SQLite database object.
@@ -334,7 +337,9 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, PyObject *database,
334337
self->initialized = 1;
335338

336339
if (autocommit == AUTOCOMMIT_DISABLED) {
337-
(void)connection_exec_stmt(self, "BEGIN");
340+
if (connection_exec_stmt(self, "BEGIN") < 0) {
341+
return -1;
342+
}
338343
}
339344
return 0;
340345

@@ -432,48 +437,83 @@ free_callback_contexts(pysqlite_Connection *self)
432437
static void
433438
remove_callbacks(sqlite3 *db)
434439
{
435-
sqlite3_trace_v2(db, SQLITE_TRACE_STMT, 0, 0);
440+
/* None of these APIs can fail, as long as they are given a valid
441+
* database pointer. */
442+
assert(db != NULL);
443+
int rc = sqlite3_trace_v2(db, SQLITE_TRACE_STMT, 0, 0);
444+
assert(rc == SQLITE_OK), (void)rc;
445+
436446
sqlite3_progress_handler(db, 0, 0, (void *)0);
437-
(void)sqlite3_set_authorizer(db, NULL, NULL);
447+
448+
rc = sqlite3_set_authorizer(db, NULL, NULL);
449+
assert(rc == SQLITE_OK), (void)rc;
438450
}
439451

440-
static void
452+
static int
441453
connection_close(pysqlite_Connection *self)
442454
{
443-
if (self->db) {
444-
if (self->autocommit == AUTOCOMMIT_DISABLED &&
445-
!sqlite3_get_autocommit(self->db))
446-
{
447-
/* If close is implicitly called as a result of interpreter
448-
* tear-down, we must not call back into Python. */
449-
if (_Py_IsInterpreterFinalizing(PyInterpreterState_Get())) {
450-
remove_callbacks(self->db);
451-
}
452-
(void)connection_exec_stmt(self, "ROLLBACK");
455+
if (self->db == NULL) {
456+
return 0;
457+
}
458+
459+
int rc = 0;
460+
if (self->autocommit == AUTOCOMMIT_DISABLED &&
461+
!sqlite3_get_autocommit(self->db))
462+
{
463+
if (connection_exec_stmt(self, "ROLLBACK") < 0) {
464+
rc = -1;
453465
}
466+
}
454467

455-
free_callback_contexts(self);
468+
sqlite3 *db = self->db;
469+
self->db = NULL;
456470

457-
sqlite3 *db = self->db;
458-
self->db = NULL;
471+
Py_BEGIN_ALLOW_THREADS
472+
/* The v2 close call always returns SQLITE_OK if given a valid database
473+
* pointer (which we do), so we can safely ignore the return value */
474+
(void)sqlite3_close_v2(db);
475+
Py_END_ALLOW_THREADS
459476

460-
Py_BEGIN_ALLOW_THREADS
461-
int rc = sqlite3_close_v2(db);
462-
assert(rc == SQLITE_OK), (void)rc;
463-
Py_END_ALLOW_THREADS
464-
}
477+
free_callback_contexts(self);
478+
return rc;
465479
}
466480

467481
static void
468-
connection_dealloc(pysqlite_Connection *self)
482+
connection_finalize(PyObject *self)
469483
{
470-
PyTypeObject *tp = Py_TYPE(self);
471-
PyObject_GC_UnTrack(self);
472-
tp->tp_clear((PyObject *)self);
484+
pysqlite_Connection *con = (pysqlite_Connection *)self;
485+
PyObject *exc = PyErr_GetRaisedException();
486+
487+
/* If close is implicitly called as a result of interpreter
488+
* tear-down, we must not call back into Python. */
489+
PyInterpreterState *interp = PyInterpreterState_Get();
490+
int teardown = _Py_IsInterpreterFinalizing(interp);
491+
if (teardown && con->db) {
492+
remove_callbacks(con->db);
493+
}
473494

474495
/* Clean up if user has not called .close() explicitly. */
475-
connection_close(self);
496+
if (connection_close(con) < 0) {
497+
if (teardown) {
498+
PyErr_Clear();
499+
}
500+
else {
501+
PyErr_WriteUnraisable((PyObject *)self);
502+
}
503+
}
504+
505+
PyErr_SetRaisedException(exc);
506+
}
476507

508+
static void
509+
connection_dealloc(PyObject *self)
510+
{
511+
if (PyObject_CallFinalizerFromDealloc(self) < 0) {
512+
return;
513+
}
514+
PyTypeObject *tp = Py_TYPE(self);
515+
PyObject_GC_UnTrack(self);
516+
tp->tp_clear(self);
477517
tp->tp_free(self);
478518
Py_DECREF(tp);
479519
}
@@ -621,7 +661,9 @@ pysqlite_connection_close_impl(pysqlite_Connection *self)
621661

622662
pysqlite_close_all_blobs(self);
623663
Py_CLEAR(self->statement_cache);
624-
connection_close(self);
664+
if (connection_close(self) < 0) {
665+
return NULL;
666+
}
625667

626668
Py_RETURN_NONE;
627669
}
@@ -2555,6 +2597,7 @@ static struct PyMemberDef connection_members[] =
25552597
};
25562598

25572599
static PyType_Slot connection_slots[] = {
2600+
{Py_tp_finalize, connection_finalize},
25582601
{Py_tp_dealloc, connection_dealloc},
25592602
{Py_tp_doc, (void *)connection_doc},
25602603
{Py_tp_methods, connection_methods},

0 commit comments

Comments
 (0)
0