From b578a4a3ddf80f5af7e287e70e39a624d4fe1b40 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Sun, 9 Oct 2022 16:54:49 +0200 Subject: [PATCH 1/6] gh-87634: deprecate cached_property locking, add lock kwarg --- Doc/library/functools.rst | 25 +++++++- Doc/whatsnew/3.12.rst | 11 ++++ Lib/functools.py | 58 ++++++++++++----- Lib/test/test_functools.py | 64 ++++++++++++++++++- ...2-10-09-16-54-08.gh-issue-87634.WfPZ2E.rst | 2 + 5 files changed, 141 insertions(+), 19 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-10-09-16-54-08.gh-issue-87634.WfPZ2E.rst diff --git a/Doc/library/functools.rst b/Doc/library/functools.rst index 2f0a9bd8be8815..1c49d9777021d1 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 @@ -110,8 +110,31 @@ The :mod:`functools` module defines the following functions: def stdev(self): return statistics.stdev(self._data) + 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 can be avoided by passing ``lock=False``:: + + class DataSet: + def __init__(self, sequence_of_numbers): + self._data = sequence_of_numbers + + @cached_property(lock=False) + def stdev(self): + return statistics.stdev(self._data) + + 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. + .. versionadded:: 3.8 + .. versionchanged:: 3.12 + The ``lock`` argument was added. + .. function:: cmp_to_key(func) diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 90353326100665..c3538efb319a7b 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. + 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/functools.py b/Lib/functools.py index 43ead512e1ea4e..c938097013dc56 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -20,6 +20,7 @@ from reprlib import recursive_repr from _thread import RLock from types import GenericAlias +from warnings import warn ################################################################################ @@ -963,13 +964,27 @@ 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 lock: + warn("Locking cached_property is deprecated; use @cached_property(lock=False)", + PendingDeprecationWarning, 2) + self.lock = RLock() if lock else None + + def __call__(self, func=None): + if self.func is not None: + raise TypeError("'cached_property' object is not callable") + self.func = func + 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 +999,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 +1014,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/test/test_functools.py b/Lib/test/test_functools.py index 730ab1f595f22c..6ebace2155e8fd 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -2946,6 +2946,21 @@ def cost(self): return self._cost +class CachedCostItemWaitNoLock: + + def __init__(self, event): + self._cost = 1 + self.lock = py_functools.RLock() + self.event = event + + @py_functools.cached_property(lock=False) + def cost(self): + self.event.wait(1) + with self.lock: + self._cost += 1 + return self._cost + + class CachedCostItemWithSlots: __slots__ = ('_cost') @@ -2970,10 +2985,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 +3003,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(PendingDeprecationWarning): + f = py_functools.cached_property(lambda s: None) + def test_object_with_slots(self): item = CachedCostItemWithSlots() with self.assertRaisesRegex( @@ -3065,6 +3097,32 @@ class Foo: ): Foo().cp + def test_call_once_completed(self): + cp = py_functools.cached_property(lambda s: None) + + 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/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. From 4e4e647980d4893ee988fb98e99841c17e25301d Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Sun, 9 Oct 2022 17:26:08 +0200 Subject: [PATCH 2/6] Fix docs --- Doc/library/functools.rst | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/Doc/library/functools.rst b/Doc/library/functools.rst index 1c49d9777021d1..bdd176d1c92e6f 100644 --- a/Doc/library/functools.rst +++ b/Doc/library/functools.rst @@ -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,18 @@ 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 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. @@ -110,26 +122,6 @@ The :mod:`functools` module defines the following functions: def stdev(self): return statistics.stdev(self._data) - 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 can be avoided by passing ``lock=False``:: - - class DataSet: - def __init__(self, sequence_of_numbers): - self._data = sequence_of_numbers - - @cached_property(lock=False) - def stdev(self): - return statistics.stdev(self._data) - - 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. - .. versionadded:: 3.8 .. versionchanged:: 3.12 From 246d72ec556f83bceaf089a7a7f0a9ee9570149f Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Sun, 9 Oct 2022 17:26:47 +0200 Subject: [PATCH 3/6] Use lock=False throughout stdlib and docs --- Doc/faq/programming.rst | 2 +- Doc/howto/descriptor.rst | 2 +- Lib/ipaddress.py | 8 ++++---- Lib/pdb.py | 2 +- Lib/platform.py | 2 +- Lib/test/test_functools.py | 6 +++--- Lib/test/test_zoneinfo/test_zoneinfo.py | 2 +- 7 files changed, 12 insertions(+), 12 deletions(-) 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/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 6ebace2155e8fd..fa61c8450b6921 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -2912,7 +2912,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,7 +2928,7 @@ 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: @@ -2967,7 +2967,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') 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: From 7cb546e6340291b335e1d75945eb561162a02137 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Sun, 9 Oct 2022 17:56:56 +0200 Subject: [PATCH 4/6] Fix tests --- Lib/ensurepip/__init__.py | 2 ++ Lib/functools.py | 5 ++++- Lib/test/test_venv.py | 5 +++-- 3 files changed, 9 insertions(+), 3 deletions(-) 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 c938097013dc56..18cd09abeb07cf 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -967,7 +967,8 @@ class cached_property: def __init__(self, func=None, *, lock=True): self.func = func self.attrname = None - self.__doc__ = func.__doc__ + if func is not None: + self.__doc__ = func.__doc__ if lock: warn("Locking cached_property is deprecated; use @cached_property(lock=False)", PendingDeprecationWarning, 2) @@ -977,6 +978,8 @@ def __call__(self, func=None): if self.func is not None: raise TypeError("'cached_property' object is not callable") self.func = func + if func is not None: + self.__doc__ = func.__doc__ return self def __set_name__(self, owner, name): diff --git a/Lib/test/test_venv.py b/Lib/test/test_venv.py index 4359a4e3ebe861..806ec1130a081a 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 From 26c26596e233a8f96eb7a0020181476538ee69f6 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Sun, 9 Oct 2022 18:12:13 +0200 Subject: [PATCH 5/6] Review comments --- Doc/library/functools.rst | 7 ++++++- Lib/functools.py | 4 ++-- Lib/test/test_functools.py | 36 ++++++++++++++++++++---------------- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/Doc/library/functools.rst b/Doc/library/functools.rst index bdd176d1c92e6f..c9505eaa917053 100644 --- a/Doc/library/functools.rst +++ b/Doc/library/functools.rst @@ -92,7 +92,8 @@ The :mod:`functools` module defines the following functions: 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 can be avoided by passing ``lock=False``. + 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 @@ -127,6 +128,10 @@ The :mod:`functools` module defines the following functions: .. 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/Lib/functools.py b/Lib/functools.py index 18cd09abeb07cf..8ad150d85335b3 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -20,7 +20,6 @@ from reprlib import recursive_repr from _thread import RLock from types import GenericAlias -from warnings import warn ################################################################################ @@ -970,13 +969,14 @@ def __init__(self, func=None, *, lock=True): if func is not None: self.__doc__ = func.__doc__ if lock: + from warnings import warn warn("Locking cached_property is deprecated; use @cached_property(lock=False)", PendingDeprecationWarning, 2) self.lock = RLock() if lock else None def __call__(self, func=None): if self.func is not None: - raise TypeError("'cached_property' object is not callable") + raise TypeError(f"{type(self).__name__!r} object is not callable") self.func = func if func is not None: self.__doc__ = func.__doc__ diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index fa61c8450b6921..e2470a1f527acb 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 @@ -2931,19 +2933,21 @@ def get_cost(self): cached_cost = py_functools.cached_property(get_cost, lock=False) -class CachedCostItemWait: +with warnings.catch_warnings(): + warnings.simplefilter("ignore", category=PendingDeprecationWarning) + class CachedCostItemWait: - def __init__(self, event): - self._cost = 1 - self.lock = py_functools.RLock() - self.event = event + 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 + @py_functools.cached_property + def cost(self): + self.event.wait(1) + with self.lock: + self._cost += 1 + return self._cost class CachedCostItemWaitNoLock: @@ -3033,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 @@ -3050,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 @@ -3065,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 @@ -3085,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 @@ -3098,7 +3102,7 @@ class Foo: Foo().cp def test_call_once_completed(self): - cp = py_functools.cached_property(lambda s: None) + cp = py_functools.cached_property(lambda s: None, lock=False) with self.assertRaisesRegex( TypeError, From ffdb4711051d809a51f5a33188ee9c89940e2745 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Sat, 15 Oct 2022 03:21:57 +0400 Subject: [PATCH 6/6] Review comments --- Doc/whatsnew/3.12.rst | 18 +++++++++--------- Lib/functools.py | 9 ++++++--- Lib/test/test_functools.py | 4 ++-- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 5cb4564139252b..ea20f21429cf7d 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -245,15 +245,15 @@ Deprecated :exc:`ImportWarning`). (Contributed by Brett Cannon in :gh:`65961`.) -* The locking behavior of :class:`~functools.cached_property` is deprecated. - 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. +* 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`.) diff --git a/Lib/functools.py b/Lib/functools.py index 8ad150d85335b3..f4b6725f79959b 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -969,9 +969,12 @@ def __init__(self, func=None, *, lock=True): if func is not None: self.__doc__ = func.__doc__ if lock: - from warnings import warn - warn("Locking cached_property is deprecated; use @cached_property(lock=False)", - PendingDeprecationWarning, 2) + 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): diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index e2470a1f527acb..f6d40500dca9af 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -2934,7 +2934,7 @@ def get_cost(self): with warnings.catch_warnings(): - warnings.simplefilter("ignore", category=PendingDeprecationWarning) + warnings.simplefilter("ignore", category=DeprecationWarning) class CachedCostItemWait: def __init__(self, event): @@ -3024,7 +3024,7 @@ def test_threaded_no_lock(self): self.assertEqual(item.cost, 4) def test_locking_deprecated(self): - with self.assertWarns(PendingDeprecationWarning): + with self.assertWarns(DeprecationWarning): f = py_functools.cached_property(lambda s: None) def test_object_with_slots(self):