[3.15] gh-146452: Improve locking granularity in pickle's batch_dict_exact and fix race condition (GH-150025) (#150039)

miss-islington · scopreon · web-flow · commit 66ade2861fec · 2026-05-18T23:53:57.000Z
gh-146452: Improve locking granularity in pickle's batch_dict_exact and fix race condition (GH-150025) Remove assertion that could fail in rare race condition. 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. (cherry picked from commit 57a0e57) Co-authored-by: Saul Cooperman <58375603+scopreon@users.noreply.github.com>
diff --git a/Misc/NEWS.d/next/Library/2026-05-18-15-30-34.gh-issue-146452.RM0EVJ.rst b/Misc/NEWS.d/next/Library/2026-05-18-15-30-34.gh-issue-146452.RM0EVJ.rst
@@ -0,0 +1,2 @@
+Fix race condition when pickling dictionaries in free threaded builds. Also
+reduce critical section cover.
diff --git a/Modules/_pickle.c b/Modules/_pickle.c
@@ -3450,6 +3450,9 @@ batch_dict(PickleState *state, PicklerObject *self, PyObject *iter, PyObject *or
  * Returns 0 on success, -1 on error.
  *
  * Note that this currently doesn't work for protocol 0.
+
+ * gh-146452: Wrap the dict iteration in a critical sections to prevent
+ * concurrent mutation from invalidating PyDict_Next() iteration state.
  */
 static int
 batch_dict_exact(PickleState *state, PicklerObject *self, PyObject *obj)
@@ -3466,15 +3469,24 @@ batch_dict_exact(PickleState *state, PicklerObject *self, PyObject *obj)
     assert(self->proto > 0);
 
     dict_size = PyDict_GET_SIZE(obj);
-    assert(dict_size);
 
     /* Write in batches of BATCHSIZE. */
     Py_ssize_t total = 0;
     do {
         if (dict_size - total == 1) {
-            PyDict_Next(obj, &ppos, &key, &value);
-            Py_INCREF(key);
-            Py_INCREF(value);
+            int next;
+            Py_BEGIN_CRITICAL_SECTION(obj);
+            next = PyDict_Next(obj, &ppos, &key, &value);
+            if (next) {
+                Py_INCREF(key);
+                Py_INCREF(value);
+            }
+            Py_END_CRITICAL_SECTION();
+            if (!next) {
+                PyErr_SetString(PyExc_RuntimeError,
+                                "dictionary changed size during iteration");
+                goto error;
+            }
             if (save(state, self, key, 0) < 0) {
                 goto error;
             }
@@ -3492,9 +3504,18 @@ batch_dict_exact(PickleState *state, PicklerObject *self, PyObject *obj)
         i = 0;
         if (_Pickler_Write(self, &mark_op, 1) < 0)
             return -1;
-        while (PyDict_Next(obj, &ppos, &key, &value)) {
-            Py_INCREF(key);
-            Py_INCREF(value);
+        int next;
+        while (1) {
+            Py_BEGIN_CRITICAL_SECTION(obj);
+            next = PyDict_Next(obj, &ppos, &key, &value);
+            if (next) {
+                Py_INCREF(key);
+                Py_INCREF(value);
+            }
+            Py_END_CRITICAL_SECTION();
+            if (!next) {
+                break;
+            }
             if (save(state, self, key, 0) < 0) {
                 goto error;
             }

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+Fix race condition when pickling dictionaries in free threaded builds. Also`
	`2`	`+reduce critical section cover.`