Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit fd19509

Browse filesBrowse 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
Copy full SHA for fd19509

File tree

Expand file treeCollapse file tree

2 files changed

+77
-31
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+77
-31
lines changed
+3Lines 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

Copy file name to clipboardExpand all lines: Modules/_sqlite/connection.c
+74-31Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.