gh-87634: remove locking from functools.cached_property (GH-101890)

Remove the undocumented locking capabilities of functools.cached_property.

diff --git a/Doc/library/functools.rst b/Doc/library/functools.rst
index 80a405e87d8d563dda9f14646240132651d38854..d467e50bc7a424c5711c38155c043ff553c10f03 100644 (file)
--- a/Doc/library/functools.rst
+++ b/Doc/library/functools.rst
@@ -86,6 +86,14 @@ 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.
 
+   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
+   locking inside the decorated getter function or around the cached property
+   access.
+
    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,6 +118,14 @@ The :mod:`functools` module defines the following functions:
            def stdev(self):
                return statistics.stdev(self._data)
 
+
+   .. versionchanged:: 3.12
+      Prior to Python 3.12, ``cached_property`` included an undocumented lock to
+      ensure that in multi-threaded usage the getter function was guaranteed to
+      run only once per instance. However, the lock was per-property, not
+      per-instance, which could result in unacceptably high lock contention. In
+      Python 3.12+ this locking is removed.
+
    .. versionadded:: 3.8
 
 

diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst
index c62f462a19a2df1e9b63795012270b5dfaba6c19..909c9102a405f37f07181858703a34283493df13 100644 (file)
--- a/Doc/whatsnew/3.12.rst
+++ b/Doc/whatsnew/3.12.rst
@@ -761,6 +761,15 @@ Changes in the Python API
   around process-global resources, which are best managed from the main interpreter.
   (Contributed by Dong-hee Na in :gh:`99127`.)
 
+* The undocumented locking behavior of :func:`~functools.cached_property`
+  is removed, because it locked across all instances of the class, leading to high
+  lock contention. This means that a cached property getter function could now run
+  more than once for a single instance, if two threads race. For most simple
+  cached properties (e.g. those that are idempotent and simply calculate a value
+  based on other attributes of the instance) this will be fine.  If
+  synchronization is needed, implement locking within the cached property getter
+  function or around multi-threaded access points.
+
 
 Build Changes
 =============

diff --git a/Lib/functools.py b/Lib/functools.py
index 43ead512e1ea4e63c2b82c4eb3f8db75d4e1eb92..aaf4291150fbbf2bd388f931d9c41d22c35b51f1 100644 (file)
--- a/Lib/functools.py
+++ b/Lib/functools.py
@@ -959,15 +959,12 @@ class singledispatchmethod:
 ### cached_property() - computed once per instance, cached as attribute
 ################################################################################
 
-_NOT_FOUND = object()
-
 
 class cached_property:
     def __init__(self, func):
         self.func = func
         self.attrname = None
         self.__doc__ = func.__doc__
-        self.lock = RLock()
 
     def __set_name__(self, owner, name):
         if self.attrname is None:
@@ -992,21 +989,15 @@ class cached_property:
                 f"instance to cache {self.attrname!r} property."
             )
             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
+        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 730ab1f595f22c7e279fd633525d48f57a6856e0..57db96d37ee369a93944947753a5789cc335bf53 100644 (file)
--- a/Lib/test/test_functools.py
+++ b/Lib/test/test_functools.py
@@ -2931,21 +2931,6 @@ class OptionallyCachedCostItem:
     cached_cost = py_functools.cached_property(get_cost)
 
 
-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 CachedCostItemWithSlots:
     __slots__ = ('_cost')
 
@@ -2970,27 +2955,6 @@ class TestCachedProperty(unittest.TestCase):
         self.assertEqual(item.get_cost(), 4)
         self.assertEqual(item.cached_cost, 3)
 
-    @threading_helper.requires_working_threading()
-    def test_threaded(self):
-        go = threading.Event()
-        item = CachedCostItemWait(go)
-
-        num_threads = 3
-
-        orig_si = sys.getswitchinterval()
-        sys.setswitchinterval(1e-6)
-        try:
-            threads = [
-                threading.Thread(target=lambda: item.cost)
-                for k in range(num_threads)
-            ]
-            with threading_helper.start_threads(threads):
-                go.set()
-        finally:
-            sys.setswitchinterval(orig_si)
-
-        self.assertEqual(item.cost, 2)
-
     def test_object_with_slots(self):
         item = CachedCostItemWithSlots()
         with self.assertRaisesRegex(

diff --git a/Misc/NEWS.d/next/Library/2023-02-13-12-55-48.gh-issue-87634.q-SBhJ.rst b/Misc/NEWS.d/next/Library/2023-02-13-12-55-48.gh-issue-87634.q-SBhJ.rst
new file mode 100644 (file)
index 0000000..a179275
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2023-02-13-12-55-48.gh-issue-87634.q-SBhJ.rst
@@ -0,0 +1 @@
+Remove locking behavior from :func:`functools.cached_property`.