Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a lock on cached_property if not present #1811

Merged
merged 2 commits into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 10 additions & 1 deletion kombu/utils/objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

from __future__ import annotations

from threading import RLock

__all__ = ('cached_property',)

try:
Expand All @@ -25,10 +27,17 @@ def __init__(self, fget=None, fset=None, fdel=None):
# This is a backport so we set this ourselves.
self.attrname = self.func.__name__

if not hasattr(self, 'lock'):
# Prior to Python 3.12, functools.cached_property has an
# undocumented lock which is required for thread-safe __set__
# and __delete__. Create one if it isn't already present.
self.lock = RLock()

def __get__(self, instance, owner=None):
# TODO: Remove this after we drop support for Python<3.8
# or fix the signature in the cached_property package
return super().__get__(instance, owner)
with self.lock:
return super().__get__(instance, owner)
tari marked this conversation as resolved.
Show resolved Hide resolved

def __set__(self, instance, value):
if instance is None:
Expand Down
32 changes: 32 additions & 0 deletions t/unit/utils/test_objects.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from __future__ import annotations

from unittest import mock

from kombu.utils.objects import cached_property


Expand Down Expand Up @@ -51,3 +53,33 @@ def foo(self, value):
assert x.xx == 10

del x.foo

def test_locks_on_access(self):

class X:
@cached_property
def foo(self):
return 42

x = X()

# Getting the value acquires the lock, and may do so recursively
# on Python < 3.12 because the superclass acquires it.
with mock.patch.object(X.foo, 'lock') as mock_lock:
assert x.foo == 42
mock_lock.__enter__.assert_called()
mock_lock.__exit__.assert_called()

# Setting a value also acquires the lock.
with mock.patch.object(X.foo, 'lock') as mock_lock:
x.foo = 314
assert x.foo == 314
mock_lock.__enter__.assert_called_once()
mock_lock.__exit__.assert_called_once()

# .. as does clearing the cached value to recompute it.
with mock.patch.object(X.foo, 'lock') as mock_lock:
del x.foo
assert x.foo == 42
mock_lock.__enter__.assert_called_once()
mock_lock.__exit__.assert_called_once()