Skip to content

Commit c07ac25

Browse files
committed
fix(cache): remove double-locking in LruCache and add tests\n\n- Make 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
1 parent 6849da7 commit c07ac25

File tree

1 file changed

+51
-51
lines changed

1 file changed

+51
-51
lines changed

Sources/Casbin/Cache/MemoryCache.swift

Lines changed: 51 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414

1515
import NIOConcurrencyHelpers
1616

17-
public final class DefaultCache:Cache {
17+
/// A simple in-memory LRU-backed cache used by the Enforcer.
18+
/// - Note: Thread-safety is provided by the underlying LruCache.
19+
public final class DefaultCache: Cache {
1820
init(lru: LruCache<Int, Bool>) {
1921
self.lru = lru
2022
}
@@ -41,7 +43,9 @@ public final class DefaultCache:Cache {
4143
}
4244
}
4345

44-
final class LruCache<Key:Hashable,Value> {
46+
/// A minimal O(1) LRU cache using a dictionary + doubly linked list.
47+
/// Operations are protected by a `NIOLock` and are safe for concurrent use.
48+
final class LruCache<Key: Hashable, Value> {
4549
private class ListNode {
4650
var key: Key?
4751
var value: Value?
@@ -53,73 +57,69 @@ final class LruCache<Key:Hashable,Value> {
5357
self.value = value
5458
}
5559
}
56-
private var storage:[Key:ListNode] = [:]
60+
private var storage: [Key: ListNode] = [:]
61+
/// Maximum number of entries. When full, the least-recently-used entry is evicted.
5762
var capacity = 0
58-
private var lock:NIOLock
63+
private var lock: NIOLock
5964

6065
/// head's nextNode is the actual first node in the Double Linked-list.
6166
private var head = ListNode()
6267
/// tail's prevNode is the actual last node in the Double Linked-list.
6368
private var tail = ListNode()
6469

6570
init(capacity: Int) {
66-
self.capacity = capacity
67-
head.nextNode = tail
68-
tail.prevNode = head
69-
self.lock = .init()
71+
self.capacity = capacity
72+
head.nextNode = tail
73+
tail.prevNode = head
74+
self.lock = .init()
75+
}
76+
/// Remove a node from the linked list and storage. Caller must hold the lock.
77+
private func removeUnlocked(_ node: ListNode) {
78+
node.prevNode?.nextNode = node.nextNode
79+
node.nextNode?.prevNode = node.prevNode
80+
if let key = node.key { storage.removeValue(forKey: key) }
7081
}
71-
/// Remove Node in the Double Linked-list.
72-
private func remove(node: ListNode) {
73-
self.lock.lock()
74-
defer { self.lock.unlock() }
75-
node.prevNode?.nextNode = node.nextNode
76-
node.nextNode?.prevNode = node.prevNode
77-
guard let key = node.key else { return }
78-
storage.removeValue(forKey: key)
79-
}
8082
func clear() {
8183
self.lock.lock()
8284
defer { self.lock.unlock() }
8385
self.storage = [:]
8486
}
85-
/// insertion is always fullfilled on the Head side.
86-
private func insertToHead(node: ListNode) {
87-
self.lock.lock()
88-
defer { self.lock.unlock() }
89-
head.nextNode?.prevNode = node
90-
node.nextNode = head.nextNode
91-
node.prevNode = head
92-
head.nextNode = node
93-
guard let key = node.key else { return }
94-
storage.updateValue(node, forKey: key)
95-
}
87+
/// Insert a node at the head (most recently used). Caller must hold the lock.
88+
private func insertToHeadUnlocked(_ node: ListNode) {
89+
head.nextNode?.prevNode = node
90+
node.nextNode = head.nextNode
91+
node.prevNode = head
92+
head.nextNode = node
93+
if let key = node.key { storage[key] = node }
94+
}
9695
/// When the cache hit happen, remove the node what you get and insert to Head side again.
97-
func getValue(forKey key: Key) -> Value? {
96+
func getValue(forKey key: Key) -> Value? {
9897
self.lock.lock()
9998
defer { self.lock.unlock() }
100-
if !storage.contains(where: { $0.key == key }) {
101-
return nil
102-
}
103-
guard let node = storage[key] else { return nil }
104-
remove(node: node)
105-
insertToHead(node: node)
106-
return node.value
107-
}
99+
guard let node = storage[key] else { return nil }
100+
removeUnlocked(node)
101+
insertToHeadUnlocked(node)
102+
return node.value
103+
}
108104
/// Push your value and if there is same value, remove that automatically.
109105
/// if not, remove Least Recently Used Node and push new node.
110-
func setValue(value: Value, forKey key: Key) {
111-
self.lock.lock()
112-
defer { self.lock.unlock() }
113-
let newNode = ListNode(key: key, value: value)
114-
if storage.contains(where: { $0.key == key }){
115-
guard let oldNode = storage[key] else { return }
116-
remove(node: oldNode)
117-
} else {
118-
if storage.count >= capacity {
119-
guard let tailNode = tail.prevNode else { return }
120-
remove(node: tailNode) // remove Least Recently Used Node
121-
}
122-
}
123-
insertToHead(node: newNode)
106+
func setValue(value: Value, forKey key: Key) {
107+
self.lock.lock()
108+
defer { self.lock.unlock() }
109+
// Update existing
110+
if let existing = storage[key] {
111+
existing.value = value
112+
removeUnlocked(existing)
113+
insertToHeadUnlocked(existing)
114+
return
124115
}
116+
// Capacity guard
117+
guard capacity > 0 else { return }
118+
// Evict if full
119+
if storage.count >= capacity, let last = tail.prevNode, last !== head {
120+
removeUnlocked(last)
121+
}
122+
let newNode = ListNode(key: key, value: value)
123+
insertToHeadUnlocked(newNode)
124+
}
125125
}

0 commit comments

Comments
 (0)