hyperoslo / Cache

:package: Nothing but Cache.
Other
2.96k stars 335 forks source link

Use the same underlying DispatchQueue for sync/async access in Storage #287

Closed simba909 closed 1 year ago

simba909 commented 3 years ago

While using this library in one of the projects I've been working on recently I came across some crashes due to race conditions and upon further inspection I noticed that Storage uses different queues for accessing shared state - thus causing race conditions.

Since the queues are used to protect shared state it makes sense to me to unify them so that all access happens on the same queue to ensure thread-safety.

Let me know what you think, happy for any feedback 🙂 👍🏻

simba909 commented 3 years ago

Did a quick search and noticed that this was introduced with #146

That PR mentions the risk of deadlocks by synchronously accessing the Storage in an async callback, which I'd argue is not a supported use-case given the current design of the library. In the case that one needs to do that, one can leave the internal queue of the Storage instance by calling .async() on another queue first:

storage.async.setObject(myObject, forKey: myKey) { _ in
  DispatchQueue.main.async {
    let anotherObject = storage.sync.object(forKey: anotherKey)
    // ...
  }
}

The only way I know of to avoid both that case and race conditions is to use locks to protect shared state, but that would incur a bigger change to the library.