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..fd565eef6683ed 100644 --- a/Include/internal/pycore_dict.h +++ b/Include/internal/pycore_dict.h @@ -27,6 +27,11 @@ 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). +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 8425f6030010f3..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: - cache[self.attrname] = val - except TypeError: + 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_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/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 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..d71af330ce3772 --- /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 once a value is cached, concurrent accesses will not overwrite it. 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)