Skip to content

Commit f380f85

Browse files
miss-islingtonoverlordekumaraditya303
authored
[3.14] gh-146452: Fix pickle segfault on concurrent mutation of dict in pickle (GH-146470) (#149940)
gh-146452: Fix pickle segfault on concurrent mutation of dict in pickle (GH-146470) (cherry picked from commit e62a611) Co-authored-by: Farhan Saif <fsaif@uic.edu> Co-authored-by: Kumar Aditya <kumaraditya@python.org>
1 parent 03d01d6 commit f380f85

3 files changed

Lines changed: 59 additions & 1 deletion

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import pickle
2+
import threading
3+
import unittest
4+
5+
from test.support import threading_helper
6+
7+
8+
@threading_helper.requires_working_threading()
9+
class TestPickleFreeThreading(unittest.TestCase):
10+
11+
def test_pickle_dumps_with_concurrent_dict_mutation(self):
12+
# gh-146452: Pickling a dict while another thread mutates it
13+
# used to segfault. batch_dict_exact() iterated dict items via
14+
# PyDict_Next() which returns borrowed references, and a
15+
# concurrent pop/replace could free the value before Py_INCREF
16+
# got to it.
17+
shared = {str(i): list(range(20)) for i in range(50)}
18+
19+
def dumper():
20+
for _ in range(1000):
21+
try:
22+
pickle.dumps(shared)
23+
except RuntimeError:
24+
# "dictionary changed size during iteration" is expected
25+
pass
26+
27+
def mutator():
28+
for j in range(1000):
29+
key = str(j % 50)
30+
shared[key] = list(range(j % 20))
31+
if j % 10 == 0:
32+
shared.pop(key, None)
33+
shared[key] = [j]
34+
35+
threads = []
36+
for _ in range(10):
37+
threads.append(threading.Thread(target=dumper))
38+
threads.append(threading.Thread(target=mutator))
39+
40+
with threading_helper.start_threads(threads):
41+
pass
42+
43+
if __name__ == "__main__":
44+
unittest.main()
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix segfault in :mod:`pickle` when pickling a dictionary concurrently
2+
mutated by another thread in the free-threaded build.

Modules/_pickle.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3353,7 +3353,7 @@ batch_dict(PickleState *state, PicklerObject *self, PyObject *iter, PyObject *or
33533353
* Note that this currently doesn't work for protocol 0.
33543354
*/
33553355
static int
3356-
batch_dict_exact(PickleState *state, PicklerObject *self, PyObject *obj)
3356+
batch_dict_exact_impl(PickleState *state, PicklerObject *self, PyObject *obj)
33573357
{
33583358
PyObject *key = NULL, *value = NULL;
33593359
int i;
@@ -3424,6 +3424,18 @@ batch_dict_exact(PickleState *state, PicklerObject *self, PyObject *obj)
34243424
return -1;
34253425
}
34263426

3427+
/* gh-146452: Wrap the dict iteration in a critical section to prevent
3428+
concurrent mutation from invalidating PyDict_Next() iteration state. */
3429+
static int
3430+
batch_dict_exact(PickleState *state, PicklerObject *self, PyObject *obj)
3431+
{
3432+
int ret;
3433+
Py_BEGIN_CRITICAL_SECTION(obj);
3434+
ret = batch_dict_exact_impl(state, self, obj);
3435+
Py_END_CRITICAL_SECTION();
3436+
return ret;
3437+
}
3438+
34273439
static int
34283440
save_dict(PickleState *state, PicklerObject *self, PyObject *obj)
34293441
{

0 commit comments

Comments
 (0)