From abb7af09854e13997071e2395aed9727c3328dc7 Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Sun, 19 Jul 2026 09:45:06 +0200 Subject: [PATCH 1/5] Make functools.cache (unbound) and cached_property strictly idempotent When running multiple threads, we don't want to guarantee call-once style behavior for functools.cache or cached_property. That could be convenient but is not strictly needed. However, we should use `dict.setdefault()` to ensure that: obj.cached_property is obj.cached_property and: cached_fuction(arg) is cached_function(arg) Both may be run twice (and for statistics purposes we may return a cached value but still have a miss). --- Doc/library/functools.rst | 15 ++- Include/internal/pycore_dict.h | 4 + Lib/functools.py | 2 +- .../test_free_threading/test_functools.py | 56 ++++++++++- Modules/_functoolsmodule.c | 12 ++- Objects/dictobject.c | 94 ++++++++++++++----- 6 files changed, 148 insertions(+), 35 deletions(-) diff --git a/Doc/library/functools.rst b/Doc/library/functools.rst index 2b46978f058102..6c2974b3bc12da 100644 --- a/Doc/library/functools.rst +++ b/Doc/library/functools.rst @@ -49,6 +49,11 @@ The :mod:`!functools` module defines the following functions: It is possible for the wrapped function to be called more than once if another thread makes an additional call before the initial call has been completed and cached. + However, even when this happens all threads will return the same object. + + .. versionchanged:: next + In earlier versions the unbound cache was not guaranteed idempotent + for threaded access. Call-once behavior is not guaranteed because locks are not held during the function call. Potentially another call with the same arguments could @@ -90,9 +95,8 @@ The :mod:`!functools` module defines the following functions: The *cached_property* does not prevent a possible race condition in multi-threaded usage. The getter function could run more than once on the - same instance, with the latest run setting the cached value. If the cached - property is idempotent or otherwise not harmful to run more than once on an - instance, this is fine. If synchronization is needed, implement the necessary + same instance, with the first run setting the cached value. + If it is necessary to guarantee work is only done once, implement the necessary locking inside the decorated getter function or around the cached property access. @@ -114,6 +118,11 @@ The :mod:`!functools` module defines the following functions: .. versionadded:: 3.8 + .. versionchanged:: next + In earlier versions the :deco:`cached_property` decorator was not guaranteed + idempotent for threaded access. For simultaneous access multiple threads + might all set the cached value. + .. versionchanged:: 3.12 Prior to Python 3.12, :deco:`!cached_property` included an undocumented lock to ensure that in multi-threaded usage the getter function was guaranteed to diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h index 7032b61d7654b8..9ab677c5b85dd5 100644 --- a/Include/internal/pycore_dict.h +++ b/Include/internal/pycore_dict.h @@ -27,6 +27,10 @@ PyAPI_FUNC(int) _PyDict_DelItemIf(PyObject *mp, PyObject *key, // Export for '_asyncio' shared extension PyAPI_FUNC(int) _PyDict_SetItem_KnownHash(PyObject *mp, PyObject *key, PyObject *item, Py_hash_t hash); +// Same return codes as PyDict_SetDefaultRef (0 inserted, 1 present, -1 error). +extern int _PyDict_SetDefaultRef_KnownHash(PyObject *mp, PyObject *key, + PyObject *default_value, + Py_hash_t hash, PyObject **result); // Export for '_asyncio' shared extension PyAPI_FUNC(int) _PyDict_DelItem_KnownHash(PyObject *mp, PyObject *key, Py_hash_t hash); diff --git a/Lib/functools.py b/Lib/functools.py index 8425f6030010f3..1f1af5f8b764f5 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -1173,7 +1173,7 @@ def __get__(self, instance, owner=None): if val is _NOT_FOUND: val = self.func(instance) try: - cache[self.attrname] = val + val = cache.setdefault(self.attrname, val) except TypeError: msg = ( f"The '__dict__' attribute on {type(instance).__name__!r} instance " diff --git a/Lib/test/test_free_threading/test_functools.py b/Lib/test/test_free_threading/test_functools.py index a442fe056cef15..eb0fc70b536b3a 100644 --- a/Lib/test/test_free_threading/test_functools.py +++ b/Lib/test/test_free_threading/test_functools.py @@ -1,7 +1,8 @@ import random +import time import unittest -from functools import lru_cache +from functools import lru_cache, cached_property from threading import Barrier, Thread from test.support import threading_helper @@ -70,6 +71,59 @@ def test_reentrant_cache_clear_unbounded(self): def test_reentrant_cache_clear_bounded(self): self._test_reentrant_cache_clear(maxsize=128) + def test_unbounded_cache_idempotent(self): + all_identical = True + + @lru_cache(maxsize=None) + def func(arg=0): + time.sleep(0.01) + return object() + + def thread_func(): + nonlocal all_identical + + for i in range(1000): + if func(i) is not func(i): + all_identical = False + + threads = [] + for _ in range(10): + t = Thread(target=thread_func) + threads.append(t) + + with threading_helper.start_threads(threads): + pass + + self.assertTrue(all_identical) + + + def test_cached_property_idempotent(self): + all_identical = True + + class C: + @cached_property + def prop(self): + time.sleep(0.01) + return object() + + c = C() + + def thread_func(): + nonlocal all_identical + + if c.prop is not c.prop: + all_identical = False + + threads = [] + for _ in range(10): + t = Thread(target=thread_func) + threads.append(t) + + with threading_helper.start_threads(threads): + pass + + self.assertTrue(all_identical) + if __name__ == "__main__": unittest.main() diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index b4595c55d519b9..b6df09865190f1 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -1329,13 +1329,15 @@ infinite_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwd Py_DECREF(key); return NULL; } - if (_PyDict_SetItem_KnownHash(self->cache, key, result, hash) < 0) { - Py_DECREF(result); - Py_DECREF(key); + PyObject *cached_value; + res = _PyDict_SetDefaultRef_KnownHash( + self->cache, key, result, hash, &cached_value); + Py_DECREF(result); + Py_DECREF(key); + if (res < 0) { return NULL; } - Py_DECREF(key); - return result; + return cached_value; } static void diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 246ffe6c18e9b5..87000af32c2a9e 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -438,6 +438,10 @@ setitem_lock_held(PyDictObject *mp, PyObject *key, PyObject *value); static int dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_value, PyObject **result, int incref_result); +static int +setdefault_ref_lock_held_known_hash(PyDictObject *mp, PyObject *key, + PyObject *default_value, Py_hash_t hash, + PyObject **result, int incref_result); #ifndef NDEBUG static int _PyObject_InlineValuesConsistencyCheck(PyObject *obj); @@ -4810,37 +4814,16 @@ dict_get_impl(PyDictObject *self, PyObject *key, PyObject *default_value) } static int -dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_value, +setdefault_ref_lock_held_known_hash(PyDictObject *mp, PyObject *key, + PyObject *default_value, Py_hash_t hash, PyObject **result, int incref_result) { - if (!PyDict_Check(d)) { - if (PyFrozenDict_Check(d)) { - frozendict_does_not_support("assignment"); - } - else { - PyErr_BadInternalCall(); - } - if (result) { - *result = NULL; - } - return -1; - } - assert(can_modify_dict((PyDictObject*)d)); + assert(can_modify_dict(mp)); + assert(hash != -1); - PyDictObject *mp = (PyDictObject *)d; PyObject *value; - Py_hash_t hash; Py_ssize_t ix; - hash = _PyObject_HashDictKey(key); - if (hash == -1) { - dict_unhashable_type(d, key); - if (result) { - *result = NULL; - } - return -1; - } - if (mp->ma_keys == Py_EMPTY_KEYS) { if (insert_to_emptydict(mp, Py_NewRef(key), hash, Py_NewRef(default_value)) < 0) { @@ -4911,6 +4894,67 @@ dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_valu return 1; } +static int +dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_value, + PyObject **result, int incref_result) +{ + if (!PyDict_Check(d)) { + if (PyFrozenDict_Check(d)) { + frozendict_does_not_support("assignment"); + } + else { + PyErr_BadInternalCall(); + } + if (result) { + *result = NULL; + } + return -1; + } + + Py_hash_t hash = _PyObject_HashDictKey(key); + if (hash == -1) { + dict_unhashable_type(d, key); + if (result) { + *result = NULL; + } + return -1; + } + + return setdefault_ref_lock_held_known_hash( + (PyDictObject *)d, key, default_value, hash, result, incref_result); +} + +int +_PyDict_SetDefaultRef_KnownHash(PyObject *d, PyObject *key, + PyObject *default_value, Py_hash_t hash, + PyObject **result) +{ + assert(key); + assert(default_value); + assert(hash != -1); + + if (!PyDict_Check(d)) { + if (PyFrozenDict_Check(d)) { + frozendict_does_not_support("assignment"); + } + else { + PyErr_BadInternalCall(); + } + if (result) { + *result = NULL; + } + return -1; + } + + int res; + Py_BEGIN_CRITICAL_SECTION(d); + res = setdefault_ref_lock_held_known_hash( + (PyDictObject *)d, key, default_value, hash, result, 1); + Py_END_CRITICAL_SECTION(); + return res; +} + + int PyDict_SetDefaultRef(PyObject *d, PyObject *key, PyObject *default_value, PyObject **result) From a65ebbbf0d4a46d82c9599e1b265ce333a72386b Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sun, 19 Jul 2026 09:01:07 +0000 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2026-07-19-09-01-02.gh-issue-150708.AESsFo.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-07-19-09-01-02.gh-issue-150708.AESsFo.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-07-19-09-01-02.gh-issue-150708.AESsFo.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-07-19-09-01-02.gh-issue-150708.AESsFo.rst new file mode 100644 index 00000000000000..50874888d0ebd9 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-07-19-09-01-02.gh-issue-150708.AESsFo.rst @@ -0,0 +1 @@ +:deco:`functools.cache` and :deco:`functools.cached_property` now guarantee an idempotent result on threaded access. This does not guarantee that the underlying function will be called exactly once, but the returned value is always the same object. From f825ab07cc9b332d75c919dc1ec61a594cdfed73 Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Sun, 19 Jul 2026 16:19:46 +0200 Subject: [PATCH 3/5] mappingproxy (and also frozendict) don't have a setdefault attribute --- Lib/functools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/functools.py b/Lib/functools.py index 1f1af5f8b764f5..2d083fa368fa24 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -1174,7 +1174,7 @@ def __get__(self, instance, owner=None): val = self.func(instance) try: val = cache.setdefault(self.attrname, val) - except TypeError: + except AttributeError: msg = ( f"The '__dict__' attribute on {type(instance).__name__!r} instance " f"does not support item assignment for caching {self.attrname!r} property." From 2f016491789a0af7bd90be39adef2ada8f4daab9 Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Wed, 29 Jul 2026 20:41:20 +0200 Subject: [PATCH 4/5] Update Misc/NEWS.d/next/Core_and_Builtins/2026-07-19-09-01-02.gh-issue-150708.AESsFo.rst Co-authored-by: Petr Viktorin --- .../2026-07-19-09-01-02.gh-issue-150708.AESsFo.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-07-19-09-01-02.gh-issue-150708.AESsFo.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-07-19-09-01-02.gh-issue-150708.AESsFo.rst index 50874888d0ebd9..d71af330ce3772 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2026-07-19-09-01-02.gh-issue-150708.AESsFo.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-07-19-09-01-02.gh-issue-150708.AESsFo.rst @@ -1 +1 @@ -:deco:`functools.cache` and :deco:`functools.cached_property` now guarantee an idempotent result on threaded access. This does not guarantee that the underlying function will be called exactly once, but the returned value is always the same object. +:deco:`functools.cache` and :deco:`functools.cached_property` now guarantee an idempotent result on threaded access. This does not guarantee that the underlying function will be called exactly once, but once a value is cached, concurrent accesses will not overwrite it. From 93902025daf80db830ede420e9b66b06500c5ce0 Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Wed, 29 Jul 2026 20:52:31 +0200 Subject: [PATCH 5/5] Address Petr's review comments --- Include/internal/pycore_dict.h | 7 ++++--- Lib/functools.py | 5 +++-- Lib/test/test_functools.py | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h index 9ab677c5b85dd5..fd565eef6683ed 100644 --- a/Include/internal/pycore_dict.h +++ b/Include/internal/pycore_dict.h @@ -28,9 +28,10 @@ PyAPI_FUNC(int) _PyDict_DelItemIf(PyObject *mp, PyObject *key, PyAPI_FUNC(int) _PyDict_SetItem_KnownHash(PyObject *mp, PyObject *key, PyObject *item, Py_hash_t hash); // Same return codes as PyDict_SetDefaultRef (0 inserted, 1 present, -1 error). -extern int _PyDict_SetDefaultRef_KnownHash(PyObject *mp, PyObject *key, - PyObject *default_value, - Py_hash_t hash, PyObject **result); +PyAPI_FUNC(int) _PyDict_SetDefaultRef_KnownHash(PyObject *mp, PyObject *key, + PyObject *default_value, + Py_hash_t hash, + PyObject **result); // Export for '_asyncio' shared extension PyAPI_FUNC(int) _PyDict_DelItem_KnownHash(PyObject *mp, PyObject *key, Py_hash_t hash); diff --git a/Lib/functools.py b/Lib/functools.py index 2d083fa368fa24..f63c76ea4acbef 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -1173,13 +1173,14 @@ def __get__(self, instance, owner=None): if val is _NOT_FOUND: val = self.func(instance) try: - val = cache.setdefault(self.attrname, val) + setdefault = cache.setdefault except AttributeError: msg = ( f"The '__dict__' attribute on {type(instance).__name__!r} instance " - f"does not support item assignment for caching {self.attrname!r} property." + f"does not support 'setdefault' for caching {self.attrname!r} property." ) raise TypeError(msg) from None + val = setdefault(self.attrname, val) return val __class_getitem__ = classmethod(GenericAlias) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index c30386afe41849..ffba63e74e2a0c 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -3723,7 +3723,7 @@ class MyClass(metaclass=MyMeta): with self.assertRaisesRegex( TypeError, - "The '__dict__' attribute on 'MyMeta' instance does not support item assignment for caching 'prop' property.", + "The '__dict__' attribute on 'MyMeta' instance does not support 'setdefault' for caching 'prop' property.", ): MyClass.prop