From 3ad075a536d177170a7a5d7b04d81e94bd5bf404 Mon Sep 17 00:00:00 2001
From: Peter Marheine <taricorp@gmail.com>
Date: Wed, 18 Oct 2023 20:45:10 +1100
Subject: [PATCH] Create a lock on cached_property if not present (#1811)

* Create a lock on cached_property if not present

This fixes #1804 (fixing breakage caused by use of undocumented
implementation details of functools.cached_property) by ensuring a lock
is always present on cached_property attributes, which is required to
safely support setting and deleting cached values in addition to
computing them on demand.

* Add a unit test for cached_property locking
---
 kombu/utils/objects.py       | 11 ++++++++++-
 t/unit/utils/test_objects.py | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/kombu/utils/objects.py b/kombu/utils/objects.py
index 737a94529..862f914b3 100644
--- a/kombu/utils/objects.py
+++ b/kombu/utils/objects.py
@@ -2,6 +2,8 @@
 
 from __future__ import annotations
 
+from threading import RLock
+
 __all__ = ('cached_property',)
 
 try:
@@ -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)
 
     def __set__(self, instance, value):
         if instance is None:
diff --git a/t/unit/utils/test_objects.py b/t/unit/utils/test_objects.py
index b9f1484a5..e2d1619ec 100644
--- a/t/unit/utils/test_objects.py
+++ b/t/unit/utils/test_objects.py
@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+from unittest import mock
+
 from kombu.utils.objects import cached_property
 
 
@@ -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()