Skip to content

Commit 8f6a3e3

Browse files
committed
Improve locking granularity in pickle's batch_dict_exact (free-threading)
Replace the coarse critical section wrapping the entire function with fine-grained sections covering only PyDict_Next + Py_INCREF. Also handle PyDict_Next returning 0 in the single-item fast path.
1 parent 5775aa8 commit 8f6a3e3

1 file changed

Lines changed: 29 additions & 20 deletions

File tree

Modules/_pickle.c

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3450,9 +3450,12 @@ batch_dict(PickleState *state, PicklerObject *self, PyObject *iter, PyObject *or
34503450
* Returns 0 on success, -1 on error.
34513451
*
34523452
* Note that this currently doesn't work for protocol 0.
3453+
3454+
* gh-146452: Wrap the dict iteration in a critical sections to prevent
3455+
* concurrent mutation from invalidating PyDict_Next() iteration state.
34533456
*/
34543457
static int
3455-
batch_dict_exact_impl(PickleState *state, PicklerObject *self, PyObject *obj)
3458+
batch_dict_exact(PickleState *state, PicklerObject *self, PyObject *obj)
34563459
{
34573460
PyObject *key = NULL, *value = NULL;
34583461
int i;
@@ -3466,15 +3469,24 @@ batch_dict_exact_impl(PickleState *state, PicklerObject *self, PyObject *obj)
34663469
assert(self->proto > 0);
34673470

34683471
dict_size = PyDict_GET_SIZE(obj);
3469-
assert(dict_size);
34703472

34713473
/* Write in batches of BATCHSIZE. */
34723474
Py_ssize_t total = 0;
34733475
do {
34743476
if (dict_size - total == 1) {
3475-
PyDict_Next(obj, &ppos, &key, &value);
3476-
Py_INCREF(key);
3477-
Py_INCREF(value);
3477+
int next;
3478+
Py_BEGIN_CRITICAL_SECTION(obj);
3479+
next = PyDict_Next(obj, &ppos, &key, &value);
3480+
if (next) {
3481+
Py_INCREF(key);
3482+
Py_INCREF(value);
3483+
}
3484+
Py_END_CRITICAL_SECTION();
3485+
if (!next) {
3486+
PyErr_SetString(PyExc_RuntimeError,
3487+
"dictionary changed size during iteration");
3488+
goto error;
3489+
}
34783490
if (save(state, self, key, 0) < 0) {
34793491
goto error;
34803492
}
@@ -3492,9 +3504,18 @@ batch_dict_exact_impl(PickleState *state, PicklerObject *self, PyObject *obj)
34923504
i = 0;
34933505
if (_Pickler_Write(self, &mark_op, 1) < 0)
34943506
return -1;
3495-
while (PyDict_Next(obj, &ppos, &key, &value)) {
3496-
Py_INCREF(key);
3497-
Py_INCREF(value);
3507+
int next;
3508+
while (1) {
3509+
Py_BEGIN_CRITICAL_SECTION(obj);
3510+
next = PyDict_Next(obj, &ppos, &key, &value);
3511+
if (next) {
3512+
Py_INCREF(key);
3513+
Py_INCREF(value);
3514+
}
3515+
Py_END_CRITICAL_SECTION();
3516+
if (!next) {
3517+
break;
3518+
}
34983519
if (save(state, self, key, 0) < 0) {
34993520
goto error;
35003521
}
@@ -3525,18 +3546,6 @@ batch_dict_exact_impl(PickleState *state, PicklerObject *self, PyObject *obj)
35253546
return -1;
35263547
}
35273548

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

0 commit comments

Comments
 (0)