gh-151633: avoid use-after-free in Counter.update

Lib/test/test_free_threading/test_collections.py

@@ -1,5 +1,5 @@
 import unittest
-from collections import deque
+from collections import Counter, deque
 from copy import copy
 from test.support import threading_helper
 
@@ -49,5 +49,17 @@ def mutate():
         )
 
 
+class TestCounter(unittest.TestCase):
+    def test_update_concurrent(self):
+        # gh-151633: concurrent Counter.update calls must not cause use-after-free
+        NTHREADS = 4
+        PER_THREAD = 5000
+        c = Counter()
+        data = ('x',) * PER_THREAD
+        threading_helper.run_concurrently(
+            c.update, nthreads=NTHREADS, args=(data,)
+        )
+
+
 if __name__ == "__main__":
     unittest.main()

Misc/NEWS.d/next/Library/2026-06-18-11-26-00.gh-issue-151633.uJzHdc.rst

@@ -0,0 +1 @@
+Fix a race condition under free-threading when multiple threads update the same :class:`~collections.Counter` concurrently.

Modules/_collectionsmodule.c

@@ -1,6 +1,6 @@
 #include "Python.h"
 #include "pycore_call.h"          // _PyObject_CallNoArgs()
-#include "pycore_dict.h"          // _PyDict_GetItem_KnownHash()
+#include "pycore_dict.h"          // _PyDict_GetItemRef_KnownHash_LockHeld()
 #include "pycore_long.h"          // _PyLong_GetZero()
 #include "pycore_moduleobject.h"  // _PyModule_GetState()
 #include "pycore_pyatomic_ft_wrappers.h"
@@ -2548,7 +2548,6 @@ _collections__count_elements_impl(PyObject *module, PyObject *mapping,
 /*[clinic end generated code: output=7e0c1789636b3d8f input=e79fad04534a0b45]*/
 {
     PyObject *it, *oldval;
-    PyObject *newval = NULL;
     PyObject *key = NULL;
     PyObject *bound_get = NULL;
     PyObject *mapping_get;
@@ -2595,24 +2594,36 @@ _collections__count_elements_impl(PyObject *module, PyObject *mapping,
                 goto done;
             }
 
-            oldval = _PyDict_GetItem_KnownHash(mapping, key, hash);
-            if (oldval == NULL) {
-                if (PyErr_Occurred())
-                    goto done;
-                if (_PyDict_SetItem_KnownHash(mapping, key, one, hash) < 0)
-                    goto done;
-            } else {
-                /* oldval is a borrowed reference.  Keep it alive across
-                   PyNumber_Add(), which can execute arbitrary user code and
-                   mutate (or even clear) the underlying dict. */
-                Py_INCREF(oldval);
+            int found;
+            int cs_err = 0;
+            PyObject *newval = NULL;
+            Py_BEGIN_CRITICAL_SECTION(mapping);
+            found = _PyDict_GetItemRef_KnownHash_LockHeld(
+                        (PyDictObject *)mapping, key, hash, &oldval);
+            if (found < 0) {
+                cs_err = -1;
+            }
+            else if (found == 0) {
+                if (_PyDict_SetItem_KnownHash_LockHeld(
+                        (PyDictObject *)mapping, key, one, hash) < 0) {
+                    cs_err = -1;
+                }
+            }
+            else {
                 newval = PyNumber_Add(oldval, one);
                 Py_DECREF(oldval);
-                if (newval == NULL)
-                    goto done;
-                if (_PyDict_SetItem_KnownHash(mapping, key, newval, hash) < 0)
-                    goto done;
-                Py_CLEAR(newval);
+                if (newval == NULL) {
+                    cs_err = -1;
+                }
+                else if (_PyDict_SetItem_KnownHash_LockHeld(
+                        (PyDictObject *)mapping, key, newval, hash) < 0) {
+                    cs_err = -1;
+                }
+            }
+            Py_END_CRITICAL_SECTION();
+            Py_CLEAR(newval);
+            if (cs_err < 0) {
+                goto done;
             }
             Py_DECREF(key);
         }
@@ -2630,6 +2641,7 @@ _collections__count_elements_impl(PyObject *module, PyObject *mapping,
             oldval = PyObject_CallFunctionObjArgs(bound_get, key, zero, NULL);
             if (oldval == NULL)
                 break;
+            PyObject *newval;
             if (oldval == zero) {
                 newval = Py_NewRef(one);
             } else {
@@ -2638,9 +2650,10 @@ _collections__count_elements_impl(PyObject *module, PyObject *mapping,
             Py_DECREF(oldval);
             if (newval == NULL)
                 break;
-            if (PyObject_SetItem(mapping, key, newval) < 0)
+            int status = PyObject_SetItem(mapping, key, newval);
+            Py_DECREF(newval);
+            if (status < 0)
                 break;
-            Py_CLEAR(newval);
             Py_DECREF(key);
         }
     }
@@ -2650,7 +2663,6 @@ _collections__count_elements_impl(PyObject *module, PyObject *mapping,
     Py_XDECREF(mapping_setitem);
     Py_DECREF(it);
     Py_XDECREF(key);
-    Py_XDECREF(newval);
     Py_XDECREF(bound_get);
     if (PyErr_Occurred())
         return NULL;