From 496ba869b0c913d4a2c8a495dcfff0670cd4eed2 Mon Sep 17 00:00:00 2001 From: Roman Varnava Date: Thu, 29 Oct 2026 14:13:25 +0300 Subject: [PATCH 1/2] test: compare two caches from two threads with the arguments swapped --- tests/mixins.py | 77 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/tests/mixins.py b/tests/mixins.py index 19907e5..33731cc 100644 --- a/tests/mixins.py +++ b/tests/mixins.py @@ -1221,6 +1221,59 @@ def worker(seed): print("ok") """ +COMPARE_WITH_ARGUMENTS_SWAPPED = """ +import faulthandler +import sys +import threading + +import cachebox + +name = sys.argv[1] +cls = getattr(cachebox, name) +# a hang prints every thread's stack before the parent's timeout +faulthandler.dump_traceback_later(50, exit=True) +# switch threads as often as possible, also inside Key.__eq__ +sys.setswitchinterval(1e-6) + + +class Key: + def __init__(self, v): + self.v = v + + def __hash__(self): + return self.v + + def __eq__(self, other): + return self.v == other.v + + +def make(): + cache = cls(0, global_ttl=60) if name == "TTLCache" else cls(0) + for i in range(200): + cache[Key(i)] = i + return cache + + +a, b = make(), make() + + +def compare(x, y): + for _ in range(300): + assert x == y + assert not x != y + + +threads = [ + threading.Thread(target=compare, args=(a, b)), + threading.Thread(target=compare, args=(b, a)), +] +for t in threads: + t.start() +for t in threads: + t.join() +print("ok") +""" + class IssuesMixin(BaseMixin): def test_issue_5(self): @@ -1302,6 +1355,30 @@ def test_lock_waiters_with_finalizers(self): done.stderr or f"exit code {done.returncode}" ) + def test_compare_with_arguments_swapped(self): + # https://github.com/awolverp/cachebox/issues/118 + # + # `a == b` locks both caches; with `b == a` running on another thread + # the locks were taken in opposite orders and both threads hung. + name = type(self.create_cache()).__name__ + + try: + done = subprocess.run( + [sys.executable, "-c", COMPARE_WITH_ARGUMENTS_SWAPPED, name], + capture_output=True, + text=True, + timeout=60, + check=False, + ) + except subprocess.TimeoutExpired: + pytest.fail( + f"{name}: a == b and b == a on two threads never returned" + ) + + assert done.stdout.strip() == "ok", ( + done.stderr or f"exit code {done.returncode}" + ) + def test_none_refcount(self): cache = self.create_cache(1000) From f6c3069db16c70e36aeb6615dc02a385e9504583 Mon Sep 17 00:00:00 2001 From: Roman Varnava Date: Thu, 29 Oct 2026 14:13:25 +0300 Subject: [PATCH 2/2] fix: lock two caches in address order in __eq__ and __ne__ (#118) --- src/policies/wrapped.rs | 25 +++++++++++++++++++++++++ src/pyclasses/cache.rs | 6 ++---- src/pyclasses/fifocache.rs | 6 ++---- src/pyclasses/lfucache.rs | 6 ++---- src/pyclasses/lrucache.rs | 6 ++---- src/pyclasses/rrcache.rs | 6 ++---- src/pyclasses/ttlcache.rs | 6 ++---- src/pyclasses/vttlcache.rs | 6 ++---- 8 files changed, 39 insertions(+), 28 deletions(-) diff --git a/src/policies/wrapped.rs b/src/policies/wrapped.rs index 3ec77b4..badc6e9 100644 --- a/src/policies/wrapped.rs +++ b/src/policies/wrapped.rs @@ -57,6 +57,31 @@ impl Wrapped

{ pyo3::sync::MutexExt::lock_py_attached(&self.inner, py) } + /// Acquires the mutexes of two different caches and returns the guards as + /// `(self, other)`. + /// + /// The mutexes are always taken in address order, so `a == b` and `b == a` + /// on two threads cannot each hold one lock and wait for the other. + // https://github.com/awolverp/cachebox/issues/118 + pub fn policy_pair_py_attached<'a>( + &'a self, + other: &'a Self, + py: pyo3::Python<'_>, + ) -> ( + parking_lot::MutexGuard<'a, P>, + parking_lot::MutexGuard<'a, P>, + ) { + debug_assert!(!std::ptr::eq(self, other)); + + if (self as *const Self) < (other as *const Self) { + let first = self.policy_py_attached(py); + (first, other.policy_py_attached(py)) + } else { + let first = other.policy_py_attached(py); + (self.policy_py_attached(py), first) + } + } + /// Acquires the mutex only if it is free, returning `None` otherwise. /// /// For callers that must never wait for the lock, such as `__traverse__`: diff --git a/src/pyclasses/cache.rs b/src/pyclasses/cache.rs index f0b9618..df4f7be 100644 --- a/src/pyclasses/cache.rs +++ b/src/pyclasses/cache.rs @@ -514,8 +514,7 @@ impl PyCache { let self_inner = slf.0.get()?; let other_inner = other.0.get()?; - let self_policy = self_inner.policy_py_attached(py); - let other_policy = other_inner.policy_py_attached(py); + let (self_policy, other_policy) = self_inner.policy_pair_py_attached(other_inner, py); self_policy.py_eq( py, @@ -537,8 +536,7 @@ impl PyCache { let self_inner = slf.0.get()?; let other_inner = other.0.get()?; - let self_policy = self_inner.policy_py_attached(py); - let other_policy = other_inner.policy_py_attached(py); + let (self_policy, other_policy) = self_inner.policy_pair_py_attached(other_inner, py); self_policy .py_eq( diff --git a/src/pyclasses/fifocache.rs b/src/pyclasses/fifocache.rs index 87deb3c..769ae0c 100644 --- a/src/pyclasses/fifocache.rs +++ b/src/pyclasses/fifocache.rs @@ -516,8 +516,7 @@ impl PyFIFOCache { let self_inner = slf.0.get()?; let other_inner = other.0.get()?; - let self_policy = self_inner.policy_py_attached(py); - let other_policy = other_inner.policy_py_attached(py); + let (self_policy, other_policy) = self_inner.policy_pair_py_attached(other_inner, py); self_policy.py_eq( py, @@ -539,8 +538,7 @@ impl PyFIFOCache { let self_inner = slf.0.get()?; let other_inner = other.0.get()?; - let self_policy = self_inner.policy_py_attached(py); - let other_policy = other_inner.policy_py_attached(py); + let (self_policy, other_policy) = self_inner.policy_pair_py_attached(other_inner, py); self_policy .py_eq( diff --git a/src/pyclasses/lfucache.rs b/src/pyclasses/lfucache.rs index a780463..8fb36c9 100644 --- a/src/pyclasses/lfucache.rs +++ b/src/pyclasses/lfucache.rs @@ -535,8 +535,7 @@ impl PyLFUCache { let self_inner = slf.0.get()?; let other_inner = other.0.get()?; - let self_policy = self_inner.policy_py_attached(py); - let other_policy = other_inner.policy_py_attached(py); + let (self_policy, other_policy) = self_inner.policy_pair_py_attached(other_inner, py); self_policy.py_eq( py, @@ -558,8 +557,7 @@ impl PyLFUCache { let self_inner = slf.0.get()?; let other_inner = other.0.get()?; - let self_policy = self_inner.policy_py_attached(py); - let other_policy = other_inner.policy_py_attached(py); + let (self_policy, other_policy) = self_inner.policy_pair_py_attached(other_inner, py); self_policy .py_eq( diff --git a/src/pyclasses/lrucache.rs b/src/pyclasses/lrucache.rs index c5e0385..3b0b4d9 100644 --- a/src/pyclasses/lrucache.rs +++ b/src/pyclasses/lrucache.rs @@ -543,8 +543,7 @@ impl PyLRUCache { let self_inner = slf.0.get()?; let other_inner = other.0.get()?; - let self_policy = self_inner.policy_py_attached(py); - let other_policy = other_inner.policy_py_attached(py); + let (self_policy, other_policy) = self_inner.policy_pair_py_attached(other_inner, py); self_policy.py_eq( py, @@ -566,8 +565,7 @@ impl PyLRUCache { let self_inner = slf.0.get()?; let other_inner = other.0.get()?; - let self_policy = self_inner.policy_py_attached(py); - let other_policy = other_inner.policy_py_attached(py); + let (self_policy, other_policy) = self_inner.policy_pair_py_attached(other_inner, py); self_policy .py_eq( diff --git a/src/pyclasses/rrcache.rs b/src/pyclasses/rrcache.rs index 90b6085..14ac518 100644 --- a/src/pyclasses/rrcache.rs +++ b/src/pyclasses/rrcache.rs @@ -516,8 +516,7 @@ impl PyRRCache { let self_inner = slf.0.get()?; let other_inner = other.0.get()?; - let self_policy = self_inner.policy_py_attached(py); - let other_policy = other_inner.policy_py_attached(py); + let (self_policy, other_policy) = self_inner.policy_pair_py_attached(other_inner, py); self_policy.py_eq( py, @@ -539,8 +538,7 @@ impl PyRRCache { let self_inner = slf.0.get()?; let other_inner = other.0.get()?; - let self_policy = self_inner.policy_py_attached(py); - let other_policy = other_inner.policy_py_attached(py); + let (self_policy, other_policy) = self_inner.policy_pair_py_attached(other_inner, py); self_policy .py_eq( diff --git a/src/pyclasses/ttlcache.rs b/src/pyclasses/ttlcache.rs index b95e5f5..1ec696e 100644 --- a/src/pyclasses/ttlcache.rs +++ b/src/pyclasses/ttlcache.rs @@ -521,8 +521,7 @@ impl PyTTLCache { let self_inner = slf.0.get()?; let other_inner = other.0.get()?; - let self_policy = self_inner.policy_py_attached(py); - let other_policy = other_inner.policy_py_attached(py); + let (self_policy, other_policy) = self_inner.policy_pair_py_attached(other_inner, py); self_policy.py_eq( py, @@ -544,8 +543,7 @@ impl PyTTLCache { let self_inner = slf.0.get()?; let other_inner = other.0.get()?; - let self_policy = self_inner.policy_py_attached(py); - let other_policy = other_inner.policy_py_attached(py); + let (self_policy, other_policy) = self_inner.policy_pair_py_attached(other_inner, py); self_policy .py_eq( diff --git a/src/pyclasses/vttlcache.rs b/src/pyclasses/vttlcache.rs index c776153..6ac2a9a 100644 --- a/src/pyclasses/vttlcache.rs +++ b/src/pyclasses/vttlcache.rs @@ -495,8 +495,7 @@ impl PyVTTLCache { let self_inner = slf.0.get()?; let other_inner = other.0.get()?; - let self_policy = self_inner.policy_py_attached(py); - let other_policy = other_inner.policy_py_attached(py); + let (self_policy, other_policy) = self_inner.policy_pair_py_attached(other_inner, py); self_policy.py_eq( py, @@ -518,8 +517,7 @@ impl PyVTTLCache { let self_inner = slf.0.get()?; let other_inner = other.0.get()?; - let self_policy = self_inner.policy_py_attached(py); - let other_policy = other_inner.policy_py_attached(py); + let (self_policy, other_policy) = self_inner.policy_pair_py_attached(other_inner, py); self_policy .py_eq(