Fixes index math being wrong in multi-threaded scenarios

Sources/Persister/Utilities/LRUCache.swift

@@ -30,11 +30,11 @@ public final class LRUCache<Key: Hashable, Value> {
     ///   - date: The date at which the item is considered expired. If `nil`, the item will never expire.
     public func write(_ item: Value, for key: Key, expiresOn date: Date?) {
 
-        if let index = keysOrderedByRecentUse.firstIndex(of: key) {
-            moveKeyToFrontOfList(key: key, atIndex: index)
+        if keysOrderedByRecentUse.contains(element: key) {
+            moveKeyToFrontOfList(key: key)
             backingStoreDictionary[key] = item
         } else {
-            keysOrderedByRecentUse.insert(key, at: keysOrderedByRecentUse.startIndex)
+            keysOrderedByRecentUse.insertAtFront(key)
             backingStoreDictionary[key] = item
         }
 
@@ -61,8 +61,8 @@ public final class LRUCache<Key: Hashable, Value> {
             return nil
         }
 
-        if let index = keysOrderedByRecentUse.firstIndex(of: key) {
-            moveKeyToFrontOfList(key: key, atIndex: index)
+        if keysOrderedByRecentUse.contains(element: key) {
+            moveKeyToFrontOfList(key: key)
         }
 
         return value
@@ -99,9 +99,9 @@ public final class LRUCache<Key: Hashable, Value> {
         }
     }
 
-    private func moveKeyToFrontOfList(key: Key, atIndex index: Int) {
-        keysOrderedByRecentUse.remove(at: index)
-        keysOrderedByRecentUse.insert(key, at: keysOrderedByRecentUse.startIndex)
+    private func moveKeyToFrontOfList(key: Key) {
+        keysOrderedByRecentUse.removeAll(where: { $0 == key} )
+        keysOrderedByRecentUse.insertAtFront(key)
     }
 
     @objc private func didReceiveMemoryWarning() {

Sources/Persister/Utilities/SynchronizedArray.swift

@@ -12,30 +12,13 @@ import Foundation
 final class SynchronizedArray<Element: Equatable> {
     private var storage: [Element] = []
     private let queue: DispatchQueue = DispatchQueue(label: "com.lickability.synchronized-array")
-
-    /// The starting index of the array.
-    var startIndex: Int {
-        get {
-            var index: Int!
-
-            queue.sync {
-                index = storage.startIndex
-            }
-
-            return index
-        }
-    }
-
+
     /// The count of the array.
     var count: Int {
         get {
-            var count: Int!
-
-            queue.sync {
-                count = storage.count
+            return queue.sync {
+                return storage.count
             }
-
-            return count
         }
     }
 
@@ -56,29 +39,12 @@ final class SynchronizedArray<Element: Equatable> {
             }
         }
         get {
-            var element: Element!
-
-            queue.sync {
-                element = storage[index]
+            return queue.sync {
+                return storage[index]
             }
-
-            return element
         }
     }
 
-    /// The first index of the provided element.
-    /// - Parameter element: The element to find the index of.
-    /// - Returns: The index of the element, if it exists within the array.
-    func firstIndex(of element: Element) -> Array.Index? {
-        var index: Int?
-
-        queue.sync {
-            index = storage.firstIndex(of: element)
-        }
-
-        return index
-    }
-
     /// Appends a new element to the end of the array.
     /// - Parameter newElement: The element to append.
     func append(newElement: Element) {
@@ -87,41 +53,21 @@ final class SynchronizedArray<Element: Equatable> {
         }
     }
 
-    /// Inserts the specified element at the index.
+    /// Inserts the specified element at the front of the array.
     /// - Parameters:
     ///   - newElement: The element to insert.
-    ///   - index: The index to insert at.
-    func insert(_ newElement: Element, at index: Int) {
+    func insertAtFront(_ newElement: Element) {
         queue.sync {
-            storage.insert(newElement, at: index)
+            storage.insert(newElement, at: storage.startIndex)
         }
     }
 
     /// Removes and returns the last element of the collection.
     /// - Returns: The last element of the collection.
     func popLast() -> Element? {
-        var element: Element?
-
-        queue.sync {
-            element = storage.popLast()
+        return queue.sync {
+            return storage.popLast()
         }
-
-        return element
-    }
-
-
-    /// Removes and returns the element at the specified position.
-    /// - Parameter index: The index of the element to remove.
-    /// - Returns: The element that was removed.
-    @discardableResult
-    func remove(at index: Int) -> Element {
-        var element: Element!
-
-        queue.sync {
-            element = storage.remove(at: index)
-        }
-
-        return element
     }
 
     /// Removes all elements from the array.
@@ -138,4 +84,13 @@ final class SynchronizedArray<Element: Equatable> {
             try storage.removeAll(where: shouldBeRemoved)
         }
     }
+
+    /// A function that determines if the array contains the provided element.
+    /// - Parameter element: The element check to check for.
+    /// - Returns: Returns a `Bool` signalling if the array contains the provided element.
+    func contains(element: Element) -> Bool {
+        return queue.sync {
+            return storage.contains(element)
+        }
+    }
 }

Tests/LRUCacheTests.swift

@@ -71,4 +71,19 @@ class LRUCacheTests: XCTestCase {
             }
         }
     }
+
+    func testMovingLastKeyDoesntCrash() {
+        let cache = LRUCache<String, String>(capacity: .limited(numberOfItems: 3))
+        let queue = DispatchQueue(label: "com.lickability.lrucachetestsqueue", attributes: [.concurrent])
+
+        for _ in 1...100000 {
+            cache.write("1", for: "1", expiresOn: nil)
+            cache.write("2", for: "2", expiresOn: nil)
+            cache.write("3", for: "3", expiresOn: nil)
+
+            queue.async {
+                cache.removeItem(for: "2")
+            }
+        }
+    }
 }