Skip to content

Conversation

@zamderax
Copy link
Contributor

@zamderax zamderax commented Oct 11, 2025

Fix LRU cache concurrency bug and add tests

What changed

  • Fixed potential deadlock in LruCache by ensuring callers hold locks instead of internal methods attempting to re-acquire them
  • Replaced O(n) list searches with O(1) dictionary lookups for better performance
  • Added guard to prevent storing nil values
  • Added comprehensive unit tests (Swift Testing) covering set/get, eviction order, and cache updates
  • Added documentation for public methods

Why

  • Previous implementation had nested locking in both public and internal methods, which could cause deadlocks under re-entrance
  • Improves both thread safety and performance

Testing

  • All tests pass locally (macOS 15, Swift 6.1.2)
  • New tests verify cache behavior including eviction order and capacity limits

Follow-ups

  • Consider exposing cache capacity via API for better observability (currently kept internal)

… internal list ops non-locking; top-level methods guard with NIOLock\n- Avoid O(n) contains checks; use dictionary directly\n- Handle capacity == 0\n- Add basic unit tests for set/get, eviction, and updates\n- Add doc comments to DefaultCache and LruCache
@hsluoyz hsluoyz changed the title fix(cache): correct LruCache locking + add tests feat(cache): correct LruCache locking + add tests Oct 11, 2025
@hsluoyz hsluoyz merged commit b584db2 into casbin:master Oct 11, 2025
4 checks passed
@hsluoyz
Copy link
Member

hsluoyz commented Oct 11, 2025

🎉 This PR is included in version 1.10.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants