Skip to content

Commit eb69460

Browse files
committed
gh-145040: Prevent closing sqlite3 connection from within a callback
Instead of detecting a closed connection after the damage has been done, prevent Connection.close() from succeeding while a SQLite callback is executing. This aligns with the SQLite C API docs, which state that applications must not close the database connection from within a callback. Add an in_callback counter to the connection object, incremented before stmt_step() and decremented after. If close() is called while the counter is positive, ProgrammingError is raised and the database connection remains open. A counter (rather than a boolean flag) is used to correctly handle nested callbacks. Also convert test docstrings to comments per reviewer feedback, and add a test for the nested callback scenario.
1 parent c7dbc7e commit eb69460

File tree

4 files changed

+59
-7
lines changed

4 files changed

+59
-7
lines changed

Lib/test/test_sqlite3/test_userfunctions.py

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,7 @@ def value(self): return 1 << 65
588588
self.cur.execute, self.query % "err_val_ret")
589589

590590
def test_close_conn_in_window_func_value(self):
591-
"""gh-145040: closing connection in window function value() callback."""
591+
# gh-145040: closing connection in window function value() callback.
592592
con = sqlite.connect(":memory:", autocommit=True)
593593
con.execute("CREATE TABLE t(x INTEGER)")
594594
con.executemany("INSERT INTO t VALUES(?)",
@@ -751,7 +751,7 @@ def test_agg_keyword_args(self):
751751
self.con.create_aggregate("test", 1, aggregate_class=AggrText)
752752

753753
def test_aggr_close_conn_in_step(self):
754-
"""Connection.close() in an aggregate step callback must not crash."""
754+
# Connection.close() in an aggregate step callback must not crash.
755755
con = sqlite.connect(":memory:", autocommit=True)
756756
cur = con.cursor()
757757
cur.execute("CREATE TABLE t(x INTEGER)")
@@ -774,8 +774,31 @@ def finalize(self):
774774
with self.assertRaisesRegex(sqlite.ProgrammingError, msg):
775775
con.execute("SELECT agg_close(x) FROM t")
776776

777+
def test_close_conn_in_nested_callback(self):
778+
# gh-145040: close() must be prevented even in nested callbacks.
779+
con = sqlite.connect(":memory:", autocommit=True)
780+
con.execute("CREATE TABLE t(x INTEGER)")
781+
for i in range(5):
782+
con.execute("INSERT INTO t VALUES(?)", (i,))
783+
784+
def outer_func(x):
785+
con.close()
786+
return x
787+
788+
def inner_func(x):
789+
return x * 10
790+
791+
con.create_function("outer_func", 1, outer_func)
792+
con.create_function("inner_func", 1, inner_func)
793+
msg = "from within a callback"
794+
with self.assertRaisesRegex(sqlite.ProgrammingError, msg):
795+
con.execute("SELECT outer_func(inner_func(x)) FROM t")
796+
# Connection must still be usable after the failed close attempt.
797+
self.assertEqual(con.execute("SELECT 1").fetchone(), (1,))
798+
con.close()
799+
777800
def test_close_conn_in_udf_during_executemany(self):
778-
"""gh-145040: closing connection in UDF during executemany."""
801+
# gh-145040: closing connection in UDF during executemany.
779802
con = sqlite.connect(":memory:", autocommit=True)
780803
con.execute("CREATE TABLE t(x)")
781804

@@ -790,7 +813,7 @@ def close_conn(x):
790813
[(i,) for i in range(10)])
791814

792815
def test_close_conn_in_progress_handler_during_iternext(self):
793-
"""gh-145040: closing connection in progress handler during iteration."""
816+
# gh-145040: closing connection in progress handler during iteration.
794817
con = sqlite.connect(":memory:", autocommit=True)
795818
con.execute("CREATE TABLE t(x)")
796819
con.executemany("INSERT INTO t VALUES(?)",
@@ -817,7 +840,7 @@ def close_progress():
817840
gc_collect()
818841

819842
def test_close_conn_in_collation_callback(self):
820-
"""gh-145040: closing connection in collation callback."""
843+
# gh-145040: closing connection in collation callback.
821844
con = sqlite.connect(":memory:", autocommit=True)
822845
con.execute("CREATE TABLE t(x TEXT)")
823846
con.executemany("INSERT INTO t VALUES(?)",

Modules/_sqlite/connection.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,8 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, PyObject *database,
302302
self->blobs = blobs;
303303
self->row_factory = Py_NewRef(Py_None);
304304
self->text_factory = Py_NewRef(&PyUnicode_Type);
305+
self->in_callback = 0;
306+
self->close_attempted_in_callback = 0;
305307
self->trace_ctx = NULL;
306308
self->progress_ctx = NULL;
307309
self->authorizer_ctx = NULL;
@@ -655,6 +657,16 @@ pysqlite_connection_close_impl(pysqlite_Connection *self)
655657
return NULL;
656658
}
657659

660+
if (self->in_callback > 0) {
661+
self->close_attempted_in_callback = 1;
662+
PyTypeObject *tp = Py_TYPE(self);
663+
pysqlite_state *state = pysqlite_get_state_by_type(tp);
664+
PyErr_SetString(state->ProgrammingError,
665+
"Cannot close the database connection "
666+
"from within a callback function.");
667+
return NULL;
668+
}
669+
658670
pysqlite_close_all_blobs(self);
659671
Py_CLEAR(self->statement_cache);
660672
if (connection_close(self) < 0) {

Modules/_sqlite/connection.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,15 @@ typedef struct
6565

6666
int initialized;
6767

68+
/* set to 1 while a SQLite callback (UDF, aggregate, progress handler,
69+
* etc.) is executing; used to prevent closing the connection from
70+
* within a callback, which is illegal per the SQLite C API docs */
71+
int in_callback;
72+
73+
/* set to 1 when close() is attempted during a callback; checked after
74+
* stmt_step() returns to raise the appropriate ProgrammingError */
75+
int close_attempted_in_callback;
76+
6877
/* thread identification of the thread the connection was created in */
6978
unsigned long thread_ident;
7079

Modules/_sqlite/cursor.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -905,8 +905,12 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation
905905
goto error;
906906
}
907907

908+
self->connection->in_callback++;
908909
rc = stmt_step(self->statement->st);
909-
if (self->connection->db == NULL) {
910+
self->connection->in_callback--;
911+
if (self->connection->close_attempted_in_callback) {
912+
self->connection->close_attempted_in_callback = 0;
913+
PyErr_Clear();
910914
PyErr_SetString(state->ProgrammingError,
911915
"Cannot close the database connection "
912916
"from within a callback function.");
@@ -1161,10 +1165,14 @@ pysqlite_cursor_iternext(PyObject *op)
11611165
if (row == NULL) {
11621166
return NULL;
11631167
}
1168+
self->connection->in_callback++;
11641169
int rc = stmt_step(stmt);
1165-
if (self->connection->db == NULL) {
1170+
self->connection->in_callback--;
1171+
if (self->connection->close_attempted_in_callback) {
1172+
self->connection->close_attempted_in_callback = 0;
11661173
Py_DECREF(row);
11671174
Py_CLEAR(self->statement);
1175+
PyErr_Clear();
11681176
PyErr_SetString(self->connection->state->ProgrammingError,
11691177
"Cannot close the database connection "
11701178
"from within a callback function.");

0 commit comments

Comments
 (0)