Skip to content

Commit

Permalink
gh-87634: remove locking from functools.cached_property (GH-101890)
Browse files Browse the repository at this point in the history
Remove the undocumented locking capabilities of functools.cached_property.
  • Loading branch information
carljm committed Feb 23, 2023
1 parent 8f64747 commit 056dfc7
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 54 deletions.
16 changes: 16 additions & 0 deletions Doc/library/functools.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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


Expand Down
9 changes: 9 additions & 0 deletions Doc/whatsnew/3.12.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
=============
Expand Down
27 changes: 9 additions & 18 deletions Lib/functools.py
Original file line number Diff line number Diff line change
Expand Up @@ -959,15 +959,12 @@ def __isabstractmethod__(self):
### 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:
Expand All @@ -992,21 +989,15 @@ def __get__(self, instance, owner=None):
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)
36 changes: 0 additions & 36 deletions Lib/test/test_functools.py
Original file line number Diff line number Diff line change
Expand Up @@ -2931,21 +2931,6 @@ def get_cost(self):
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')

Expand All @@ -2970,27 +2955,6 @@ 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):
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(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove locking behavior from :func:`functools.cached_property`.

0 comments on commit 056dfc7

Please sign in to comment.