Skip to content

Commit 4b379b4

Browse files
committed
Fix deadlock issue
1 parent 7f714d3 commit 4b379b4

File tree

2 files changed

+129
-2
lines changed

2 files changed

+129
-2
lines changed

Sources/Cache/Cache/PersistableCache.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,12 @@ public class PersistableCache<
151151
*/
152152
public func save() throws {
153153
lock.lock()
154+
defer { lock.unlock() }
155+
154156
let persistedValues = allValues.compactMapValues(persistedValueMap)
155157
let json = JSON<Key>(initialValues: persistedValues)
156158
let data = try json.data()
157159
try data.write(to: url.fileURL(withName: name))
158-
lock.unlock()
159160
}
160161

161162
/**
@@ -165,8 +166,9 @@ public class PersistableCache<
165166
*/
166167
public func delete() throws {
167168
lock.lock()
169+
defer { lock.unlock() }
170+
168171
try FileManager.default.removeItem(at: url.fileURL(withName: name))
169-
lock.unlock()
170172
}
171173
}
172174

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
#if !os(Linux) && !os(Windows)
2+
import XCTest
3+
@testable import Cache
4+
5+
/// Tests designed to expose bugs in the Cache package
6+
final class BugExposingTests: XCTestCase {
7+
8+
// MARK: - Bug #1: PersistableCache Deadlock on Error
9+
10+
/// This test exposes a deadlock bug in PersistableCache.
11+
/// When delete() throws an error, the NSLock is never released.
12+
/// Any subsequent call to save() or delete() will hang forever.
13+
func testPersistableCacheDeadlockAfterDeleteError() {
14+
enum Key: String {
15+
case test
16+
}
17+
18+
let uniqueName = "deadlock_test_\(UUID().uuidString)"
19+
let cache: PersistableCache<Key, String, String> = PersistableCache(name: uniqueName)
20+
21+
// First: delete() on non-existent file throws - but lock is never released!
22+
XCTAssertThrowsError(try cache.delete())
23+
24+
// Now try to save - this should work but will DEADLOCK
25+
// because the lock was never released after the error above
26+
cache[.test] = "value"
27+
28+
// Use a timeout to detect the deadlock
29+
let expectation = XCTestExpectation(description: "save() should complete")
30+
31+
DispatchQueue.global().async {
32+
do {
33+
try cache.save()
34+
expectation.fulfill()
35+
} catch {
36+
expectation.fulfill() // Even if it fails, at least it didn't hang
37+
}
38+
}
39+
40+
// Wait only 2 seconds - if it hangs, we have a deadlock
41+
let result = XCTWaiter.wait(for: [expectation], timeout: 2.0)
42+
43+
// If this times out, the bug is confirmed
44+
XCTAssertNotEqual(result, .timedOut, "DEADLOCK DETECTED: save() hung after delete() error - lock was never released!")
45+
}
46+
47+
// MARK: - LRUCache contains() Updates Recency
48+
49+
/// This test validates that contains() updates LRU order.
50+
/// Calling contains() is considered a "use" of the key, promoting it to most recently used.
51+
func testLRUCacheContainsUpdatesRecency() {
52+
let cache = LRUCache<String, Int>(capacity: 2)
53+
54+
// Add items in order: a, then b
55+
cache["a"] = 1 // Order: [a]
56+
cache["b"] = 2 // Order: [a, b]
57+
58+
// At this point, "a" is the oldest (least recently used)
59+
// Calling contains() promotes "a" to most recently used
60+
let exists = cache.contains("a") // Order: [b, a]
61+
XCTAssertTrue(exists)
62+
63+
// Now add "c" - this evicts "b" (now the oldest)
64+
cache["c"] = 3
65+
66+
// contains() promoted "a", so "b" was evicted
67+
XCTAssertNotNil(cache["a"], "'a' should exist - contains() promoted it to most recently used")
68+
XCTAssertNil(cache["b"], "'b' should be evicted as the oldest after contains() promoted 'a'")
69+
XCTAssertNotNil(cache["c"])
70+
}
71+
72+
// MARK: - Bug #3: LRUCache Dual Lock Issue
73+
74+
/// This test attempts to expose race conditions from LRUCache having
75+
/// its own lock separate from the parent Cache's lock.
76+
func testLRUCacheDualLockRaceCondition() {
77+
let cache = LRUCache<Int, Int>(capacity: 50)
78+
let iterations = 100_000
79+
var errors: [String] = []
80+
let errorLock = NSLock()
81+
82+
let expectation = XCTestExpectation(description: "Concurrent operations complete")
83+
84+
DispatchQueue.concurrentPerform(iterations: iterations) { i in
85+
let key = i % 30
86+
87+
// Interleave different operations
88+
switch i % 4 {
89+
case 0:
90+
cache[key] = i
91+
case 1:
92+
_ = cache[key]
93+
case 2:
94+
_ = cache.contains(key)
95+
case 3:
96+
cache.remove(key)
97+
default:
98+
break
99+
}
100+
101+
// Periodically check consistency
102+
if i % 1000 == 0 {
103+
let count = cache.allValues.count
104+
if count > 50 {
105+
errorLock.lock()
106+
errors.append("Cache exceeded capacity: \(count) > 50 at iteration \(i)")
107+
errorLock.unlock()
108+
}
109+
}
110+
}
111+
112+
expectation.fulfill()
113+
wait(for: [expectation], timeout: 30.0)
114+
115+
// Report any consistency violations
116+
if !errors.isEmpty {
117+
XCTFail("Race condition detected! Errors:\n\(errors.joined(separator: "\n"))")
118+
}
119+
120+
// Final consistency check
121+
let finalCount = cache.allValues.count
122+
XCTAssertLessThanOrEqual(finalCount, 50, "Final cache size \(finalCount) exceeds capacity of 50")
123+
}
124+
}
125+
#endif

0 commit comments

Comments
 (0)