diff --git a/Doc/faq/programming.rst b/Doc/faq/programming.rst index ad281f826dd42c..db13fcf306d027 100644 --- a/Doc/faq/programming.rst +++ b/Doc/faq/programming.rst @@ -1939,7 +1939,7 @@ This example shows the various techniques:: # Do not cache this because old results # can be out of date. - @cached_property + @cached_property(lock=False) def location(self): "Return the longitude/latitude coordinates of the station" # Result only depends on the station_id diff --git a/Doc/howto/descriptor.rst b/Doc/howto/descriptor.rst index 74710d9b3fc2ed..b08be250622e8e 100644 --- a/Doc/howto/descriptor.rst +++ b/Doc/howto/descriptor.rst @@ -1500,7 +1500,7 @@ instance dictionary to function correctly: class CP: __slots__ = () # Eliminates the instance dict - @cached_property # Requires an instance dict + @cached_property(lock=False) # Requires an instance dict def pi(self): return 4 * sum((-1.0)**n / (2.0*n + 1.0) for n in reversed(range(100_000))) diff --git a/Doc/library/functools.rst b/Doc/library/functools.rst index 2f0a9bd8be8815..c9505eaa917053 100644 --- a/Doc/library/functools.rst +++ b/Doc/library/functools.rst @@ -55,7 +55,7 @@ The :mod:`functools` module defines the following functions: .. versionadded:: 3.9 -.. decorator:: cached_property(func) +.. decorator:: cached_property(func, *, lock=True) Transform a method of a class into a property whose value is computed once and then cached as a normal attribute for the life of the instance. Similar @@ -69,7 +69,7 @@ The :mod:`functools` module defines the following functions: def __init__(self, sequence_of_numbers): self._data = tuple(sequence_of_numbers) - @cached_property + @cached_property(lock=False) def stdev(self): return statistics.stdev(self._data) @@ -86,6 +86,19 @@ The :mod:`functools` module defines the following functions: The cached value can be cleared by deleting the attribute. This allows the *cached_property* method to run again. + By default, a ``cached_property`` instance contains an + :class:`~threading.RLock` which prevents multiple threads from executing the + cached property concurrently. This was intended to ensure that the cached + property would execute only once per instance, but it also prevents + concurrent execution on different instances of the same class, causing + unnecessary and excessive serialization of updates. This locking behavior is + deprecated and will be removed in Python 3.14. It can be avoided by passing + ``lock=False``. + + With ``lock=False``, ``cached_property`` provides no guarantee that it will + run only once per instance, if accessed in multiple threads. An application + wanting such guarantees must handle its own locking explicitly. + Note, this decorator interferes with the operation of :pep:`412` key-sharing dictionaries. This means that instance dictionaries can take more space than usual. @@ -112,6 +125,13 @@ The :mod:`functools` module defines the following functions: .. versionadded:: 3.8 + .. versionchanged:: 3.12 + The ``lock`` argument was added. + + .. deprecated-removed:: 3.12 3.14 + The locking behavior of ``cached_property`` is deprecated and will be + removed in 3.14. Use ``lock=False`` to avoid the deprecated behavior. + .. function:: cmp_to_key(func) diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index ebc490691e308b..ea20f21429cf7d 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -245,6 +245,17 @@ Deprecated :exc:`ImportWarning`). (Contributed by Brett Cannon in :gh:`65961`.) +* The locking behavior of :class:`~functools.cached_property` is deprecated and + will be removed in Python 3.14. The locking is class-wide and thus + inefficient if many instances of the same class with a cached property are + used across multiple threads (only a single instance across all instances of + the class can update its cached property at once). Integrating locking into + :class:`~functools.cached_property` was a mistake; the application should + determine its need for synchronization guarantees and lock as needed to meet + them. Use ``@cached_property(lock=False)`` to create a cached property with + no locking and avoid the deprecation warning. + (Contributed by Carl Meyer in :gh:`87634`.) + Pending Removal in Python 3.13 ------------------------------ diff --git a/Lib/ensurepip/__init__.py b/Lib/ensurepip/__init__.py index 33c91801b0aa85..639cb03a5c5d86 100644 --- a/Lib/ensurepip/__init__.py +++ b/Lib/ensurepip/__init__.py @@ -94,6 +94,8 @@ def _run_pip(args, additional_paths=None): sys.executable, '-W', 'ignore::DeprecationWarning', + '-W', + 'ignore::PendingDeprecationWarning', '-c', code, ] diff --git a/Lib/functools.py b/Lib/functools.py index 43ead512e1ea4e..f4b6725f79959b 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -963,13 +963,34 @@ def __isabstractmethod__(self): class cached_property: - def __init__(self, func): + def __init__(self, func=None, *, lock=True): self.func = func self.attrname = None - self.__doc__ = func.__doc__ - self.lock = RLock() + if func is not None: + self.__doc__ = func.__doc__ + if lock: + import warnings + warnings._deprecated( + "lock=True default behavior of cached_property", + "Locking cached_property is deprecated; use @cached_property(lock=False)", + remove=(3, 14) + ) + self.lock = RLock() if lock else None + + def __call__(self, func=None): + if self.func is not None: + raise TypeError(f"{type(self).__name__!r} object is not callable") + self.func = func + if func is not None: + self.__doc__ = func.__doc__ + return self def __set_name__(self, owner, name): + if self.func is None: + raise TypeError( + "Cannot assign cached_property(lock=...) directly to instance, " + "it must decorate a function." + ) if self.attrname is None: self.attrname = name elif name != self.attrname: @@ -984,6 +1005,11 @@ def __get__(self, instance, owner=None): if self.attrname is None: raise TypeError( "Cannot use cached_property instance without calling __set_name__ on it.") + if self.func is None: + raise TypeError( + "Cannot assign cached_property(lock=...) directly to instance, " + "it must decorate a function." + ) try: cache = instance.__dict__ except AttributeError: # not all objects have __dict__ (e.g. class defines slots) @@ -994,19 +1020,27 @@ def __get__(self, instance, owner=None): raise TypeError(msg) from None val = cache.get(self.attrname, _NOT_FOUND) if val is _NOT_FOUND: - with self.lock: - # check if another thread filled cache while we awaited lock - val = cache.get(self.attrname, _NOT_FOUND) - if val is _NOT_FOUND: - val = self.func(instance) - try: - cache[self.attrname] = val - except TypeError: - msg = ( - f"The '__dict__' attribute on {type(instance).__name__!r} instance " - f"does not support item assignment for caching {self.attrname!r} property." - ) - raise TypeError(msg) from None + if self.lock is None: + val = self._fill(instance, cache) + else: + with self.lock: + # check if another thread filled cache while we awaited lock + val = cache.get(self.attrname, _NOT_FOUND) + if val is _NOT_FOUND: + val = self._fill(instance, cache) + return val + + def _fill(self, instance, cache): + val = self.func(instance) + try: + cache[self.attrname] = val + except TypeError: + msg = ( + f"The '__dict__' attribute on {type(instance).__name__!r} instance " + f"does not support item assignment for caching {self.attrname!r} property." + ) + raise TypeError(msg) from None return val + __class_getitem__ = classmethod(GenericAlias) diff --git a/Lib/ipaddress.py b/Lib/ipaddress.py index 3f15601e700d68..36bf6b1defd096 100644 --- a/Lib/ipaddress.py +++ b/Lib/ipaddress.py @@ -755,12 +755,12 @@ def overlaps(self, other): other.network_address in self or ( other.broadcast_address in self))) - @functools.cached_property + @functools.cached_property(lock=False) def broadcast_address(self): return self._address_class(int(self.network_address) | int(self.hostmask)) - @functools.cached_property + @functools.cached_property(lock=False) def hostmask(self): return self._address_class(int(self.netmask) ^ self._ALL_ONES) @@ -1390,7 +1390,7 @@ def __init__(self, address): self.netmask = self.network.netmask self._prefixlen = self.network._prefixlen - @functools.cached_property + @functools.cached_property(lock=False) def hostmask(self): return self.network.hostmask @@ -2094,7 +2094,7 @@ def __init__(self, address): self.netmask = self.network.netmask self._prefixlen = self.network._prefixlen - @functools.cached_property + @functools.cached_property(lock=False) def hostmask(self): return self.network.hostmask diff --git a/Lib/pdb.py b/Lib/pdb.py index b0acb1f571d3f6..a2cc3fdac034a2 100755 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -175,7 +175,7 @@ def check(self): traceback.print_exc() sys.exit(1) - @functools.cached_property + @functools.cached_property(lock=False) def _details(self): import runpy return runpy._get_module_details(self) diff --git a/Lib/platform.py b/Lib/platform.py index 9f5b317287530b..902d0960c29389 100755 --- a/Lib/platform.py +++ b/Lib/platform.py @@ -847,7 +847,7 @@ class uname_result( except when needed. """ - @functools.cached_property + @functools.cached_property(lock=False) def processor(self): return _unknown_as_blank(_Processor.get()) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index 730ab1f595f22c..f6d40500dca9af 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -13,12 +13,14 @@ import typing import unittest import unittest.mock +import warnings import weakref import gc from weakref import proxy import contextlib from inspect import Signature +from test.support.warnings_helper import ignore_warnings from test.support import import_helper from test.support import threading_helper @@ -2912,7 +2914,7 @@ class CachedCostItem: def __init__(self): self.lock = py_functools.RLock() - @py_functools.cached_property + @py_functools.cached_property(lock=False) def cost(self): """The cost of the item.""" with self.lock: @@ -2928,17 +2930,34 @@ def get_cost(self): self._cost += 1 return self._cost - cached_cost = py_functools.cached_property(get_cost) + cached_cost = py_functools.cached_property(get_cost, lock=False) -class CachedCostItemWait: +with warnings.catch_warnings(): + warnings.simplefilter("ignore", category=DeprecationWarning) + class CachedCostItemWait: + + def __init__(self, event): + self._cost = 1 + self.lock = py_functools.RLock() + self.event = event + + @py_functools.cached_property + def cost(self): + self.event.wait(1) + with self.lock: + self._cost += 1 + return self._cost + + +class CachedCostItemWaitNoLock: def __init__(self, event): self._cost = 1 self.lock = py_functools.RLock() self.event = event - @py_functools.cached_property + @py_functools.cached_property(lock=False) def cost(self): self.event.wait(1) with self.lock: @@ -2952,7 +2971,7 @@ class CachedCostItemWithSlots: def __init__(self): self._cost = 1 - @py_functools.cached_property + @py_functools.cached_property(lock=False) def cost(self): raise RuntimeError('never called, slots not supported') @@ -2970,10 +2989,9 @@ def test_cached_attribute_name_differs_from_func_name(self): self.assertEqual(item.get_cost(), 4) self.assertEqual(item.cached_cost, 3) - @threading_helper.requires_working_threading() - def test_threaded(self): + def _run_threads(self, item_cls): go = threading.Event() - item = CachedCostItemWait(go) + item = item_cls(go) num_threads = 3 @@ -2989,8 +3007,26 @@ def test_threaded(self): finally: sys.setswitchinterval(orig_si) + return item + + @threading_helper.requires_working_threading() + def test_threaded(self): + item = self._run_threads(CachedCostItemWait) + + # cached_property locks, cost is only incremented once self.assertEqual(item.cost, 2) + @threading_helper.requires_working_threading() + def test_threaded_no_lock(self): + item = self._run_threads(CachedCostItemWaitNoLock) + + # cached_property does not lock, cost is incremented 3 times + self.assertEqual(item.cost, 4) + + def test_locking_deprecated(self): + with self.assertWarns(DeprecationWarning): + f = py_functools.cached_property(lambda s: None) + def test_object_with_slots(self): item = CachedCostItemWithSlots() with self.assertRaisesRegex( @@ -3001,7 +3037,7 @@ def test_object_with_slots(self): def test_immutable_dict(self): class MyMeta(type): - @py_functools.cached_property + @py_functools.cached_property(lock=False) def prop(self): return True @@ -3018,7 +3054,7 @@ def test_reuse_different_names(self): """Disallow this case because decorated function a would not be cached.""" with self.assertRaises(RuntimeError) as ctx: class ReusedCachedProperty: - @py_functools.cached_property + @py_functools.cached_property(lock=False) def a(self): pass @@ -3033,7 +3069,7 @@ def test_reuse_same_name(self): """Reusing a cached_property on different classes under the same name is OK.""" counter = 0 - @py_functools.cached_property + @py_functools.cached_property(lock=False) def _cp(_self): nonlocal counter counter += 1 @@ -3053,7 +3089,7 @@ class B: self.assertEqual(a.cp, 1) def test_set_name_not_called(self): - cp = py_functools.cached_property(lambda s: None) + cp = py_functools.cached_property(lambda s: None, lock=False) class Foo: pass @@ -3065,6 +3101,32 @@ class Foo: ): Foo().cp + def test_call_once_completed(self): + cp = py_functools.cached_property(lambda s: None, lock=False) + + with self.assertRaisesRegex( + TypeError, + "'cached_property' object is not callable", + ): + cp() + + def test_assign_to_instance_incomplete(self): + with self.assertRaisesRegex(RuntimeError, "Error calling __set_name__") as cm: + class C: + f = py_functools.cached_property(lock=False) + self.assertIn("must decorate a function", str(cm.exception.__cause__)) + + def test_assign_to_instance_incomplete_bypass_set_name(self): + class C: pass + f = py_functools.cached_property(lock=False) + # unlikely edge case, but we want to validate existence of `func` in `__get__`, + # and this requires bypassing `__set_name__` but still having `attrname` set + f.attrname = "f" + C.f = f + + with self.assertRaisesRegex(TypeError, "must decorate a function"): + C().f + def test_access_from_class(self): self.assertIsInstance(CachedCostItem.cost, py_functools.cached_property) diff --git a/Lib/test/test_venv.py b/Lib/test/test_venv.py index b355b0050cd55b..3c889b5e4d70b6 100644 --- a/Lib/test/test_venv.py +++ b/Lib/test/test_venv.py @@ -599,8 +599,8 @@ def do_test_with_pip(self, system_site_packages): envpy = os.path.join(os.path.realpath(self.env_dir), self.bindir, self.exe) # Ignore DeprecationWarning since pip code is not part of Python out, err = check_output([envpy, '-W', 'ignore::DeprecationWarning', - '-W', 'ignore::ImportWarning', '-I', - '-m', 'pip', '--version']) + '-W', 'ignore::ImportWarning', '-W', 'ignore::PendingDeprecationWarning', + '-I', '-m', 'pip', '--version']) # We force everything to text, so unittest gives the detailed diff # if we get unexpected results err = err.decode("latin-1") # Force to text, prevent decoding errors @@ -621,6 +621,7 @@ def do_test_with_pip(self, system_site_packages): envvars["PYTHONWARNINGS"] = "ignore" out, err = check_output([envpy, '-W', 'ignore::DeprecationWarning', + '-W', 'ignore::PendingDeprecationWarning', '-W', 'ignore::ImportWarning', '-I', '-m', 'ensurepip._uninstall']) # We force everything to text, so unittest gives the detailed diff diff --git a/Lib/test/test_zoneinfo/test_zoneinfo.py b/Lib/test/test_zoneinfo/test_zoneinfo.py index fd0e3bc032ec0c..16bdb6f0631600 100644 --- a/Lib/test/test_zoneinfo/test_zoneinfo.py +++ b/Lib/test/test_zoneinfo/test_zoneinfo.py @@ -1661,7 +1661,7 @@ class TestModule(ZoneInfoTestBase): def zoneinfo_data(self): return ZONEINFO_DATA - @cached_property + @cached_property(lock=False) def _UTC_bytes(self): zone_file = self.zoneinfo_data.path_from_key("UTC") with open(zone_file, "rb") as f: diff --git a/Misc/NEWS.d/next/Library/2022-10-09-16-54-08.gh-issue-87634.WfPZ2E.rst b/Misc/NEWS.d/next/Library/2022-10-09-16-54-08.gh-issue-87634.WfPZ2E.rst new file mode 100644 index 00000000000000..d4e58db4b092a8 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-10-09-16-54-08.gh-issue-87634.WfPZ2E.rst @@ -0,0 +1,2 @@ +Deprecate the locking behavior of :class:`~functools.cached_property` and +add the *lock* keyword argument.