Merge pull request #1628 from onevcat/fix/cache-crash

Fix cache crash when no disk space left

Sources/Cache/DiskStorage.swift

@@ -50,36 +50,44 @@ public enum DiskStorage {
         var maybeCached : Set<String>?
         let maybeCachedCheckingQueue = DispatchQueue(label: "com.onevcat.Kingfisher.maybeCachedCheckingQueue")
 
+        // `false` if the storage initialized with an error. This prevents unexpected forcibly crash when creating
+        // storage in the default cache.
+        private var storageReady: Bool = true
+
         /// Creates a disk storage with the given `DiskStorage.Config`.
         ///
         /// - Parameter config: The config used for this disk storage.
         /// - Throws: An error if the folder for storage cannot be got or created.
-        public init(config: Config) throws {
+        public convenience init(config: Config) throws {
+            self.init(noThrowConfig: config, creatingDirectory: false)
+            try prepareDirectory()
+        }
 
-            self.config = config
+        // If `creatingDirectory` is `false`, the directory preparation will be skipped.
+        // We need to call `prepareDirectory` manually after this returns.
+        init(noThrowConfig config: Config, creatingDirectory: Bool) {
+            var config = config
 
-            let url: URL
-            if let directory = config.directory {
-                url = directory
-            } else {
-                url = try config.fileManager.url(
-                    for: .cachesDirectory,
-                    in: .userDomainMask,
-                    appropriateFor: nil,
-                    create: true)
-            }
+            let creation = Creation(config)
+            self.directoryURL = creation.directoryURL
 
-            let cacheName = "com.onevcat.Kingfisher.ImageCache.\(config.name)"
-            directoryURL = config.cachePathBlock(url, cacheName)
+            // Break any possible retain cycle set by outside.
+            config.cachePathBlock = nil
+            self.config = config
 
-            metaChangingQueue = DispatchQueue(label: cacheName)
+            metaChangingQueue = DispatchQueue(label: creation.cacheName)
+            setupCacheChecking()
 
-            try prepareDirectory()
+            if creatingDirectory {
+                try? prepareDirectory()
+            }
+        }
 
+        private func setupCacheChecking() {
             maybeCachedCheckingQueue.async {
                 do {
                     self.maybeCached = Set()
-                    try config.fileManager.contentsOfDirectory(atPath: self.directoryURL.path).forEach { fileName in
+                    try self.config.fileManager.contentsOfDirectory(atPath: self.directoryURL.path).forEach { fileName in
                         self.maybeCached?.insert(fileName)
                     }
                 } catch {
@@ -91,7 +99,7 @@ public enum DiskStorage {
         }
 
         // Creates the storage folder.
-        func prepareDirectory() throws {
+        private func prepareDirectory() throws {
             let fileManager = config.fileManager
             let path = directoryURL.path
 
@@ -103,6 +111,7 @@ public enum DiskStorage {
                     withIntermediateDirectories: true,
                     attributes: nil)
             } catch {
+                self.storageReady = false
                 throw KingfisherError.cacheError(reason: .cannotCreateDirectory(path: path, error: error))
             }
         }
@@ -112,6 +121,10 @@ public enum DiskStorage {
             forKey key: String,
             expiration: StorageExpiration? = nil) throws
         {
+            guard storageReady else {
+                throw KingfisherError.cacheError(reason: .diskStorageIsNotReady(cacheURL: directoryURL))
+            }
+
             let expiration = expiration ?? config.expiration
             // The expiration indicates that already expired, no need to store.
             guard !expiration.isExpired else { return }
@@ -167,6 +180,10 @@ public enum DiskStorage {
             actuallyLoad: Bool,
             extendingExpiration: ExpirationExtending) throws -> T?
         {
+            guard storageReady else {
+                throw KingfisherError.cacheError(reason: .diskStorageIsNotReady(cacheURL: directoryURL))
+            }
+
             let fileManager = config.fileManager
             let fileURL = cacheFileURL(forKey: key)
             let filePath = fileURL.path
@@ -491,3 +508,21 @@ extension DiskStorage {
     }
 }
 
+extension DiskStorage {
+    struct Creation {
+        let directoryURL: URL
+        let cacheName: String
+
+        init(_ config: Config) {
+            let url: URL
+            if let directory = config.directory {
+                url = directory
+            } else {
+                url = config.fileManager.urls(for: .cachesDirectory, in: .userDomainMask)[0]
+            }
+
+            cacheName = "com.onevcat.Kingfisher.ImageCache.\(config.name)"
+            directoryURL = config.cachePathBlock(url, cacheName)
+        }
+    }
+}

Sources/Cache/ImageCache.swift

@@ -147,6 +147,7 @@ open class ImageCache {
     /// for any of your customize cache.
     public static let `default` = ImageCache(name: "default")
 
+
     // MARK: Public Properties
     /// The `MemoryStorage.Backend` object used in this cache. This storage holds loaded images in memory with a
     /// reasonable expire duration and a maximum memory usage. To modify the configuration of a storage, just set
@@ -213,7 +214,7 @@ open class ImageCache {
     ///                   You should not use the same `name` for different caches, otherwise, the disk storage would
     ///                   be conflicting to each other. The `name` should not be an empty string.
     public convenience init(name: String) {
-        try! self.init(name: name, cacheDirectoryURL: nil, diskCachePathClosure: nil)
+        self.init(noThrowName: name, cacheDirectoryURL: nil, diskCachePathClosure: nil)
     }
 
     /// Creates an `ImageCache` with a given `name`, cache directory `path`
@@ -233,17 +234,55 @@ open class ImageCache {
     public convenience init(
         name: String,
         cacheDirectoryURL: URL?,
-        diskCachePathClosure: DiskCachePathClosure? = nil) throws
+        diskCachePathClosure: DiskCachePathClosure? = nil
+    ) throws
+    {
+        if name.isEmpty {
+            fatalError("[Kingfisher] You should specify a name for the cache. A cache with empty name is not permitted.")
+        }
+
+        let memoryStorage = ImageCache.createMemoryStorage()
+
+        let config = ImageCache.createConfig(
+            name: name, cacheDirectoryURL: cacheDirectoryURL, diskCachePathClosure: diskCachePathClosure
+        )
+        let diskStorage = try DiskStorage.Backend<Data>(config: config)
+        self.init(memoryStorage: memoryStorage, diskStorage: diskStorage)
+    }
+
+    convenience init(
+        noThrowName name: String,
+        cacheDirectoryURL: URL?,
+        diskCachePathClosure: DiskCachePathClosure?
+    )
     {
         if name.isEmpty {
             fatalError("[Kingfisher] You should specify a name for the cache. A cache with empty name is not permitted.")
         }
 
+        let memoryStorage = ImageCache.createMemoryStorage()
+
+        let config = ImageCache.createConfig(
+            name: name, cacheDirectoryURL: cacheDirectoryURL, diskCachePathClosure: diskCachePathClosure
+        )
+        let diskStorage = DiskStorage.Backend<Data>(noThrowConfig: config, creatingDirectory: true)
+        self.init(memoryStorage: memoryStorage, diskStorage: diskStorage)
+    }
+
+    private static func createMemoryStorage() -> MemoryStorage.Backend<KFCrossPlatformImage> {
         let totalMemory = ProcessInfo.processInfo.physicalMemory
         let costLimit = totalMemory / 4
         let memoryStorage = MemoryStorage.Backend<KFCrossPlatformImage>(config:
             .init(totalCostLimit: (costLimit > Int.max) ? Int.max : Int(costLimit)))
+        return memoryStorage
+    }
 
+    private static func createConfig(
+        name: String,
+        cacheDirectoryURL: URL?,
+        diskCachePathClosure: DiskCachePathClosure? = nil
+    ) -> DiskStorage.Config
+    {
         var diskConfig = DiskStorage.Config(
             name: name,
             sizeLimit: 0,
@@ -252,10 +291,7 @@ open class ImageCache {
         if let closure = diskCachePathClosure {
             diskConfig.cachePathBlock = closure
         }
-        let diskStorage = try DiskStorage.Backend<Data>(config: diskConfig)
-        diskConfig.cachePathBlock = nil
-
-        self.init(memoryStorage: memoryStorage, diskStorage: diskStorage)
+        return diskConfig
     }
     
     deinit {

Sources/General/KingfisherError.swift

@@ -157,6 +157,14 @@ public enum KingfisherError: Error {
         /// - error: The underlying error originally thrown by Foundation when setting the `attributes` to the disk
         ///          file at `filePath`.
         case cannotSetCacheFileAttribute(filePath: String, attributes: [FileAttributeKey : Any], error: Error)
+
+        /// The disk storage of cache is not ready. Code 3011.
+        ///
+        /// This is usually due to extremely lack of space on disk storage, and
+        /// Kingfisher failed even when creating the cache folder. The disk storage will be in unusable state. Normally,
+        /// ask user to free some spaces and restart the app to make the disk storage work again.
+        /// - cacheURL: The intended URL which should be the storage folder.
+        case diskStorageIsNotReady(cacheURL: URL)
     }
     
     
@@ -382,6 +390,9 @@ extension KingfisherError.CacheErrorReason {
         case .cannotSetCacheFileAttribute(let filePath, let attributes, let error):
             return "Cannot set file attribute for the cache file at path: \(filePath), attributes: \(attributes)." +
                    "Underlying foundation error: \(error)."
+        case .diskStorageIsNotReady(let cacheURL):
+            return "The disk storage is not ready to use yet at URL: '\(cacheURL)'. " +
+                "This is usually caused by extremely lack of disk space. Ask users to free up some space and restart the app."
         }
     }
     
@@ -397,6 +408,7 @@ extension KingfisherError.CacheErrorReason {
         case .cannotSerializeImage: return 3008
         case .cannotCreateCacheFile: return 3009
         case .cannotSetCacheFileAttribute: return 3010
+        case .diskStorageIsNotReady: return 3011
         }
     }
 }