diff --git a/Lib/test/test_free_threading/test_collections.py b/Lib/test/test_free_threading/test_collections.py index 849b0480e232fc2..0f3432dabe9ebc0 100644 --- a/Lib/test/test_free_threading/test_collections.py +++ b/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() diff --git a/Misc/NEWS.d/next/Library/2026-06-18-11-26-00.gh-issue-151633.uJzHdc.rst b/Misc/NEWS.d/next/Library/2026-06-18-11-26-00.gh-issue-151633.uJzHdc.rst new file mode 100644 index 000000000000000..89fdf33965179ef --- /dev/null +++ b/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. diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 5ca6362406a78b9..5dac3a9804db8b6 100644 --- a/Modules/_collectionsmodule.c +++ b/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;