Skip to content

Commit 08500b2

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 08500b2

1 file changed

Lines changed: 29 additions & 19 deletions

File tree

Modules/_pickle.c

Lines changed: 29 additions & 19 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;
@@ -3472,9 +3475,19 @@ batch_dict_exact_impl(PickleState *state, PicklerObject *self, PyObject *obj)
34723475
Py_ssize_t total = 0;
34733476
do {
34743477
if (dict_size - total == 1) {
3475-
PyDict_Next(obj, &ppos, &key, &value);
3476-
Py_INCREF(key);
3477-
Py_INCREF(value);
3478+
int next;
3479+
Py_BEGIN_CRITICAL_SECTION(obj);
3480+
next = PyDict_Next(obj, &ppos, &key, &value);
3481+
if (next) {
3482+
Py_INCREF(key);
3483+
Py_INCREF(value);
3484+
}
3485+
Py_END_CRITICAL_SECTION();
3486+
if (!next) {
3487+
PyErr_SetString(PyExc_RuntimeError,
3488+
"dictionary changed size during iteration");
3489+
goto error;
3490+
}
34783491
if (save(state, self, key, 0) < 0) {
34793492
goto error;
34803493
}
@@ -3492,9 +3505,18 @@ batch_dict_exact_impl(PickleState *state, PicklerObject *self, PyObject *obj)
34923505
i = 0;
34933506
if (_Pickler_Write(self, &mark_op, 1) < 0)
34943507
return -1;
3495-
while (PyDict_Next(obj, &ppos, &key, &value)) {
3496-
Py_INCREF(key);
3497-
Py_INCREF(value);
3508+
int next;
3509+
while (1) {
3510+
Py_BEGIN_CRITICAL_SECTION(obj);
3511+
next = PyDict_Next(obj, &ppos, &key, &value);
3512+
if (next) {
3513+
Py_INCREF(key);
3514+
Py_INCREF(value);
3515+
}
3516+
Py_END_CRITICAL_SECTION();
3517+
if (!next) {
3518+
break;
3519+
}
34983520
if (save(state, self, key, 0) < 0) {
34993521
goto error;
35003522
}
@@ -3525,18 +3547,6 @@ batch_dict_exact_impl(PickleState *state, PicklerObject *self, PyObject *obj)
35253547
return -1;
35263548
}
35273549

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-
35403550
static int
35413551
save_dict(PickleState *state, PicklerObject *self, PyObject *obj)
35423552
{

0 commit comments

Comments
 (0)