Skip to content

Commit 8f55561

Browse files
committed
gh-146452: Fix pickle segfault when pickling dict with concurrent mutation
1 parent 0e54305 commit 8f55561

File tree

3 files changed

+66
-1
lines changed

3 files changed

+66
-1
lines changed
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
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+
for t in threads:
41+
t.start()
42+
for t in threads:
43+
t.join()
44+
45+
# If we get here without a segfault, the test passed.
46+
47+
48+
if __name__ == "__main__":
49+
unittest.main()
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix segfault in :mod:`pickle` when pickling a dictionary concurrently
2+
mutated by another thread in the free-threaded build. The dict iteration in
3+
``batch_dict_exact`` now holds a critical section to prevent borrowed
4+
references from being invalidated mid-iteration.

Modules/_pickle.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3451,7 +3451,7 @@ batch_dict(PickleState *state, PicklerObject *self, PyObject *iter, PyObject *or
34513451
* Note that this currently doesn't work for protocol 0.
34523452
*/
34533453
static int
3454-
batch_dict_exact(PickleState *state, PicklerObject *self, PyObject *obj)
3454+
batch_dict_exact_impl(PickleState *state, PicklerObject *self, PyObject *obj)
34553455
{
34563456
PyObject *key = NULL, *value = NULL;
34573457
int i;
@@ -3524,6 +3524,18 @@ batch_dict_exact(PickleState *state, PicklerObject *self, PyObject *obj)
35243524
return -1;
35253525
}
35263526

3527+
/* gh-146452: Wrap the dict iteration in a critical section to prevent
3528+
concurrent mutation from invalidating PyDict_Next() iteration state. */
3529+
static int
3530+
batch_dict_exact(PickleState *state, PicklerObject *self, PyObject *obj)
3531+
{
3532+
int ret;
3533+
Py_BEGIN_CRITICAL_SECTION(obj);
3534+
ret = batch_dict_exact_impl(state, self, obj);
3535+
Py_END_CRITICAL_SECTION();
3536+
return ret;
3537+
}
3538+
35273539
static int
35283540
save_dict(PickleState *state, PicklerObject *self, PyObject *obj)
35293541
{

0 commit comments

Comments
 (0)