amosavian / FileProvider

FileManager replacement for Local, iCloud and Remote (WebDAV/FTP/Dropbox/OneDrive) files -- Swift
MIT License
56 stars 14 forks source link

Possible Thread Safe Issue #187

Open Shouheng88 opened 2 months ago

Shouheng88 commented 2 months ago

This is a good library and help me a lot. When I use this library in my project I meet a thread safe issue.

If the library is used in multi thread environment it may cause EXC_BAD_ACCESS error in code:

// https://github.com/amosavian/FileProvider/blob/master/Sources/RemoteSession.swift

internal var completionHandlersForTasks = [String: [Int: SimpleCompletionHandler]]()
internal var downloadCompletionHandlersForTasks = [String: [Int: (URL) -> Void]]()
internal var dataCompletionHandlersForTasks = [String: [Int: (Data) -> Void]]()
internal var responseCompletionHandlersForTasks = [String: [Int: (URLResponse) -> Void]]()

internal func removeSessionHandler(for uuid: String) {
    _ = completionHandlersForTasks.removeValue(forKey: uuid)
    _ = downloadCompletionHandlersForTasks.removeValue(forKey: uuid)
    _ = dataCompletionHandlersForTasks.removeValue(forKey: uuid) // Here
    _ = responseCompletionHandlersForTasks.removeValue(forKey: uuid)
}

The reason is the Dictionary data structure is not thread safe in Swift. The dataCompletionHandlersForTasks is global, so it has the risk of causing EXC_BAD_ACCESS error.

Since for most of cases we call FileProvider in main thread, and all options to dataCompletionHandlersForTasks happens on main thread, so we don't have the crash. But in multi-thread environment it occurs sometimes.

I think it may not be a good design to use a global data structure to hold callbacks. We could use an instance to hold them and pass it to SessionDelegate. For example,

public class SessionHandlerHolder {

    var completionHandlersForTasks = [String: [Int: SimpleCompletionHandler]]()
    var downloadCompletionHandlersForTasks = [String: [Int: (URL) -> Void]]()
    var dataCompletionHandlersForTasks = [String: [Int: (Data) -> Void]]()
    var responseCompletionHandlersForTasks = [String: [Int: (URLResponse) -> Void]]()

    func initEmptySessionHandler(_ uuid: String) {
        completionHandlersForTasks[uuid] = [:]
        downloadCompletionHandlersForTasks[uuid] = [:]
        dataCompletionHandlersForTasks[uuid] = [:]
        responseCompletionHandlersForTasks[uuid] = [:]
    }

    func removeSessionHandler(for uuid: String) {
        _ = completionHandlersForTasks.removeValue(forKey: uuid)
        _ = downloadCompletionHandlersForTasks.removeValue(forKey: uuid)
        _ = dataCompletionHandlersForTasks.removeValue(forKey: uuid)
        _ = responseCompletionHandlersForTasks.removeValue(forKey: uuid)
    }
}

I hope this issue could make this library better! Thanks!