Skip to content

Commit e6f0c98

Browse files
committed
gh-161065: Fix sqlite3 deadlock on connections shared between threads
SQLite calls Python callbacks with the connection mutex held, so never block on that mutex while holding the GIL: enter it first, releasing the GIL only if it is contended.
1 parent e5f1477 commit e6f0c98

7 files changed

Lines changed: 288 additions & 32 deletions

File tree

‎Lib/test/test_sqlite3/test_dbapi.py‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import sqlite3 as sqlite
2727
import subprocess
2828
import sys
29+
import textwrap
2930
import threading
3031
import unittest
3132
import urllib.parse
@@ -1734,6 +1735,115 @@ def run(con, err):
17341735
t.join()
17351736
self.assertEqual(len(err), 0, "\n".join(err))
17361737

1738+
@requires_subprocess()
1739+
@unittest.skipUnless(sqlite.threadsafety == 3,
1740+
"requires SQLite in serialized threading mode")
1741+
def test_shared_connection_callback_deadlock(self):
1742+
# Threads sharing a connection must not deadlock when SQLite calls
1743+
# back into Python. A deadlock would hang the whole process, so run
1744+
# this in a child process.
1745+
SCRIPT = textwrap.dedent("""
1746+
import itertools
1747+
import sqlite3
1748+
import sys
1749+
import threading
1750+
1751+
sys.setswitchinterval(0.00001)
1752+
1753+
class Count:
1754+
def __init__(self):
1755+
self.n = 0
1756+
def step(self, x):
1757+
self.n += 1
1758+
def finalize(self):
1759+
return self.n
1760+
1761+
def connect():
1762+
cx = sqlite3.connect(":memory:", check_same_thread=False)
1763+
cx.execute("create table t(x)")
1764+
cx.executemany("insert into t values(?)",
1765+
((str(i),) for i in range(50)))
1766+
return cx
1767+
1768+
def query(cx, sql, n, errors):
1769+
# Use different SQL in each thread and for each iteration:
1770+
# the threads must not share a cached statement, and the
1771+
# authorizer is only called when a statement is prepared.
1772+
try:
1773+
for i in range(n, n + 100):
1774+
params = ("", 100) if i % 2 else [None, 100]
1775+
cx.execute(f"{sql} limit ?+{i}", params).fetchall()
1776+
except Exception as exc:
1777+
errors.append(exc)
1778+
1779+
def run(name, sql, setup, concurrent=None):
1780+
print(name, flush=True)
1781+
cx = connect()
1782+
setup(cx)
1783+
errors = []
1784+
threads = [threading.Thread(target=query,
1785+
args=(cx, sql, 1000, errors))]
1786+
if concurrent is None:
1787+
threads.append(threading.Thread(
1788+
target=query, args=(cx, sql, 2000, errors)))
1789+
for t in threads:
1790+
t.start()
1791+
while concurrent is not None and threads[0].is_alive():
1792+
concurrent(cx)
1793+
for t in threads:
1794+
t.join()
1795+
cx.close()
1796+
if errors:
1797+
raise errors[0]
1798+
1799+
def function(cx):
1800+
cx.create_function("f", 1, lambda x: x)
1801+
def collation(cx):
1802+
cx.create_collation("c", lambda a, b: (a > b) - (a < b))
1803+
1804+
run("function", "select f(x) from t where x is not ?", function)
1805+
run("aggregate", "select agg(x) from t where x is not ? group by x",
1806+
lambda cx: cx.create_aggregate("agg", 1, Count))
1807+
run("collation",
1808+
"select x from t where x is not ? order by x collate c",
1809+
collation)
1810+
run("authorizer", "select x from t where x is not ?",
1811+
lambda cx: cx.set_authorizer(lambda *a: sqlite3.SQLITE_OK))
1812+
run("progress handler", "select x from t where x is not ?",
1813+
lambda cx: cx.set_progress_handler(lambda: 0, 1))
1814+
run("trace callback", "select x from t where x is not ?",
1815+
lambda cx: cx.set_trace_callback(lambda stmt: None))
1816+
1817+
# Register callbacks while another thread is in a callback.
1818+
# Use new names: existing functions and collations cannot be
1819+
# modified while statements are running.
1820+
sql = "select f(x) from t where x is not ?"
1821+
names = map("name{}".format, itertools.count())
1822+
run("create_function", sql, function,
1823+
lambda cx: cx.create_function(next(names), 0, lambda: None))
1824+
run("create_aggregate", sql, function,
1825+
lambda cx: cx.create_aggregate(next(names), 1, Count))
1826+
run("create_collation", sql, function,
1827+
lambda cx: cx.create_collation(next(names), lambda a, b: 0))
1828+
run("set_authorizer", sql, function,
1829+
lambda cx: cx.set_authorizer(None))
1830+
run("set_progress_handler", sql, function,
1831+
lambda cx: cx.set_progress_handler(None, 0))
1832+
run("set_trace_callback", sql, function,
1833+
lambda cx: cx.set_trace_callback(None))
1834+
run("setconfig", sql, function,
1835+
lambda cx: cx.setconfig(sqlite3.SQLITE_DBCONFIG_ENABLE_FKEY))
1836+
print("done")
1837+
""")
1838+
try:
1839+
proc = subprocess.run([sys.executable, "-c", SCRIPT],
1840+
capture_output=True, text=True,
1841+
timeout=SHORT_TIMEOUT)
1842+
except subprocess.TimeoutExpired as exc:
1843+
self.fail(f"deadlock; output: {exc.stdout!r}")
1844+
self.assertEqual(proc.returncode, 0, proc.stderr)
1845+
self.assertEndsWith(proc.stdout, "done\n")
1846+
17371847

17381848
class ConstructorTests(unittest.TestCase):
17391849
def test_date(self):
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix a deadlock in :mod:`sqlite3` when a connection created with
2+
``check_same_thread=False`` is used from several threads at once and has a
3+
Python callback registered, such as a user-defined function, aggregate,
4+
collation, authorizer, progress handler or trace callback.

‎Modules/_sqlite/connection.c‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,13 +434,15 @@ remove_callbacks(sqlite3 *db)
434434
/* None of these APIs can fail, as long as they are given a valid
435435
* database pointer. */
436436
assert(db != NULL);
437+
pysqlite_db_mutex_enter(db);
437438
int rc = sqlite3_trace_v2(db, SQLITE_TRACE_STMT, 0, 0);
438439
assert(rc == SQLITE_OK), (void)rc;
439440

440441
sqlite3_progress_handler(db, 0, 0, (void *)0);
441442

442443
rc = sqlite3_set_authorizer(db, NULL, NULL);
443444
assert(rc == SQLITE_OK), (void)rc;
445+
pysqlite_db_mutex_leave(db);
444446
}
445447

446448
static int
@@ -1231,11 +1233,13 @@ pysqlite_connection_create_function_impl(pysqlite_Connection *self,
12311233
if (ctx == NULL) {
12321234
return NULL;
12331235
}
1236+
pysqlite_db_mutex_enter(self->db);
12341237
rc = sqlite3_create_function_v2(self->db, name, narg, flags, ctx,
12351238
func_callback,
12361239
NULL,
12371240
NULL,
12381241
&destructor_callback); // will free 'ctx'
1242+
pysqlite_db_mutex_leave(self->db);
12391243

12401244
if (rc != SQLITE_OK) {
12411245
/* Workaround for SQLite bug: no error code or string is available here */
@@ -1376,21 +1380,25 @@ create_window_function_impl(pysqlite_Connection *self, PyTypeObject *cls,
13761380
int flags = SQLITE_UTF8;
13771381
int rc;
13781382
if (Py_IsNone(aggregate_class)) {
1383+
pysqlite_db_mutex_enter(self->db);
13791384
rc = sqlite3_create_window_function(self->db, name, num_params, flags,
13801385
0, 0, 0, 0, 0, 0);
1386+
pysqlite_db_mutex_leave(self->db);
13811387
}
13821388
else {
13831389
callback_context *ctx = create_callback_context(cls, aggregate_class);
13841390
if (ctx == NULL) {
13851391
return NULL;
13861392
}
1393+
pysqlite_db_mutex_enter(self->db);
13871394
rc = sqlite3_create_window_function(self->db, name, num_params, flags,
13881395
ctx,
13891396
&step_callback,
13901397
&final_callback,
13911398
&value_callback,
13921399
&inverse_callback,
13931400
&destructor_callback);
1401+
pysqlite_db_mutex_leave(self->db);
13941402
}
13951403

13961404
if (rc != SQLITE_OK) {
@@ -1435,11 +1443,13 @@ pysqlite_connection_create_aggregate_impl(pysqlite_Connection *self,
14351443
if (ctx == NULL) {
14361444
return NULL;
14371445
}
1446+
pysqlite_db_mutex_enter(self->db);
14381447
rc = sqlite3_create_function_v2(self->db, name, n_arg, SQLITE_UTF8, ctx,
14391448
0,
14401449
&step_callback,
14411450
&final_callback,
14421451
&destructor_callback); // will free 'ctx'
1452+
pysqlite_db_mutex_leave(self->db);
14431453
if (rc != SQLITE_OK) {
14441454
/* Workaround for SQLite bug: no error code or string is available here */
14451455
PyErr_SetString(self->OperationalError, "Error creating aggregate");
@@ -1593,15 +1603,19 @@ pysqlite_connection_set_authorizer_impl(pysqlite_Connection *self,
15931603

15941604
int rc;
15951605
if (callable == Py_None) {
1606+
pysqlite_db_mutex_enter(self->db);
15961607
rc = sqlite3_set_authorizer(self->db, NULL, NULL);
1608+
pysqlite_db_mutex_leave(self->db);
15971609
set_callback_context(&self->authorizer_ctx, NULL);
15981610
}
15991611
else {
16001612
callback_context *ctx = create_callback_context(cls, callable);
16011613
if (ctx == NULL) {
16021614
return NULL;
16031615
}
1616+
pysqlite_db_mutex_enter(self->db);
16041617
rc = sqlite3_set_authorizer(self->db, authorizer_callback, ctx);
1618+
pysqlite_db_mutex_leave(self->db);
16051619
set_callback_context(&self->authorizer_ctx, ctx);
16061620
}
16071621
if (rc != SQLITE_OK) {
@@ -1644,15 +1658,19 @@ pysqlite_connection_set_progress_handler_impl(pysqlite_Connection *self,
16441658

16451659
if (callable == Py_None) {
16461660
/* None clears the progress handler previously set */
1661+
pysqlite_db_mutex_enter(self->db);
16471662
sqlite3_progress_handler(self->db, 0, 0, (void*)0);
1663+
pysqlite_db_mutex_leave(self->db);
16481664
set_callback_context(&self->progress_ctx, NULL);
16491665
}
16501666
else {
16511667
callback_context *ctx = create_callback_context(cls, callable);
16521668
if (ctx == NULL) {
16531669
return NULL;
16541670
}
1671+
pysqlite_db_mutex_enter(self->db);
16551672
sqlite3_progress_handler(self->db, n, progress_callback, ctx);
1673+
pysqlite_db_mutex_leave(self->db);
16561674
set_callback_context(&self->progress_ctx, ctx);
16571675
}
16581676
Py_RETURN_NONE;
@@ -1687,15 +1705,19 @@ pysqlite_connection_set_trace_callback_impl(pysqlite_Connection *self,
16871705
* - https://sqlite.org/c3ref/c_trace.html
16881706
* - https://sqlite.org/c3ref/trace_v2.html
16891707
*/
1708+
pysqlite_db_mutex_enter(self->db);
16901709
sqlite3_trace_v2(self->db, SQLITE_TRACE_STMT, 0, 0);
1710+
pysqlite_db_mutex_leave(self->db);
16911711
set_callback_context(&self->trace_ctx, NULL);
16921712
}
16931713
else {
16941714
callback_context *ctx = create_callback_context(cls, callable);
16951715
if (ctx == NULL) {
16961716
return NULL;
16971717
}
1718+
pysqlite_db_mutex_enter(self->db);
16981719
sqlite3_trace_v2(self->db, SQLITE_TRACE_STMT, trace_callback, ctx);
1720+
pysqlite_db_mutex_leave(self->db);
16991721
set_callback_context(&self->trace_ctx, ctx);
17001722
}
17011723

@@ -1728,7 +1750,9 @@ pysqlite_connection_enable_load_extension_impl(pysqlite_Connection *self,
17281750
return NULL;
17291751
}
17301752

1753+
pysqlite_db_mutex_enter(self->db);
17311754
rc = sqlite3_enable_load_extension(self->db, onoff);
1755+
pysqlite_db_mutex_leave(self->db);
17321756

17331757
if (rc != SQLITE_OK) {
17341758
PyErr_SetString(self->OperationalError,
@@ -1767,7 +1791,9 @@ pysqlite_connection_load_extension_impl(pysqlite_Connection *self,
17671791
return NULL;
17681792
}
17691793

1794+
pysqlite_db_mutex_enter(self->db);
17701795
rc = sqlite3_load_extension(self->db, extension_name, entrypoint, &errmsg);
1796+
pysqlite_db_mutex_leave(self->db);
17711797
if (rc != 0) {
17721798
PyErr_SetString(self->OperationalError, errmsg);
17731799
return NULL;
@@ -2232,8 +2258,10 @@ pysqlite_connection_create_collation_impl(pysqlite_Connection *self,
22322258
int rc;
22332259
int flags = SQLITE_UTF8;
22342260
if (callable == Py_None) {
2261+
pysqlite_db_mutex_enter(self->db);
22352262
rc = sqlite3_create_collation_v2(self->db, name, flags,
22362263
NULL, NULL, NULL);
2264+
pysqlite_db_mutex_leave(self->db);
22372265
}
22382266
else {
22392267
if (!PyCallable_Check(callable)) {
@@ -2244,9 +2272,11 @@ pysqlite_connection_create_collation_impl(pysqlite_Connection *self,
22442272
if (ctx == NULL) {
22452273
return NULL;
22462274
}
2275+
pysqlite_db_mutex_enter(self->db);
22472276
rc = sqlite3_create_collation_v2(self->db, name, flags, ctx,
22482277
&collation_callback,
22492278
&destructor_callback);
2279+
pysqlite_db_mutex_leave(self->db);
22502280
}
22512281

22522282
if (rc != SQLITE_OK) {
@@ -2570,7 +2600,9 @@ setconfig_impl(pysqlite_Connection *self, int op, int enable)
25702600
}
25712601

25722602
int actual;
2603+
pysqlite_db_mutex_enter(self->db);
25732604
int rc = sqlite3_db_config(self->db, op, enable, &actual);
2605+
pysqlite_db_mutex_leave(self->db);
25742606
if (rc != SQLITE_OK) {
25752607
set_error_from_db(self->state, self->db);
25762608
return NULL;
@@ -2605,7 +2637,9 @@ getconfig_impl(pysqlite_Connection *self, int op)
26052637
}
26062638

26072639
int current;
2640+
pysqlite_db_mutex_enter(self->db);
26082641
int rc = sqlite3_db_config(self->db, op, -1, &current);
2642+
pysqlite_db_mutex_leave(self->db);
26092643
if (rc != SQLITE_OK) {
26102644
set_error_from_db(self->state, self->db);
26112645
return -1;

0 commit comments

Comments
 (0)