swift-dependencies

swift-dependencies copied to clipboard

I tried to make a unit test for the deadlock behavior that was discussed in the point free slack. I recently had a deadlock issue in a side project with PersistentContainer and the printing debug reducer.

I explained the issue here.

I think this test replicates the issue I was running into.

1. A dependency A gets resolved on a non-main thread and acquires the CachedValues.lock
2. Another dependency B gets resolved on the main thread and wants to acquire the CachedValues.lock
3. The test, live, or previewValue default value initializer however needs synchronous main thread access

I think a solution might be to change the locking in CachedValues:

 func value<Key: TestDependencyKey>(
    for key: Key.Type,
    context: DependencyContext,
    file: StaticString = #file,
    function: StaticString = #function,
    line: UInt = #line
  ) -> Key.Value where Key.Value: Sendable {
    self.lock.lock()
    defer { self.lock.unlock() }
    // ...
    switch context {
          case .live:
            value = _liveValue(key) as? Key.Value
          case .preview:
            value = Key.previewValue
          case .test:
            value = Key.testValue
          }
    // ...

When executing the live|preview|testValue do we need hold the lock? I think it's expected that this is only run once and then cached. But this run once behavior could be perhaps implemented with a different locking mechanism compared to the self.cached mutations, which need to stay isolated of course. I'm not sure here and need to study the code more on this though.

Or all dependencies must be designed to not synchronously access the main thread, when the default value is executed. If this is a requirement it should be documented. This is how @tgrapperon fixed the PersistentContainer dependency.

I hope this is helpful.

Another approach is to document a dependency as requiring the main thread so users either eagerly resolve and initialize the dependency on app launch or in a way that's guaranteed to be on the main thread.

Perhaps there could be also a way to annotate such dependencies with something like a MainThreadDependency type. I'm not sure, if this is safe in all cases, but it fixes the test and passes all other tests on my mac.

public protocol MainThreadDependency {}
struct NeedsMainThreadDep: TestDependencyKey, MainThreadDependency {}

func value<Key: TestDependencyKey>(
    for key: Key.Type,
    context: DependencyContext,
    file: StaticString = #file,
    function: StaticString = #function,
    line: UInt = #line
  ) -> Key.Value where Key.Value: Sendable {
    if key is MainThreadDependency.Type, !Thread.isMainThread {
      return DispatchQueue.mainSync {
        return self.value(for: key, context: context)
      }
    }
    self.lock.lock()
    defer { self.lock.unlock() }
    // ...
}

Hi @KaiOelfke, thanks so much for the repro! We need a bit more time to look at this in depth, but hope to get to it later this week or next.

Any updates related to that Issue? I think I run into the same situation of my App randomly not responding because it gets stuck in exact that line of code.

Hi @SebastianBoldt, no update right now. I haven't looked into this for awhile, but if I remember correctly I don't think there is much we can do in the library other than document the behavior. It seems when this happens it is because dependencies are making assumptions about what thread it is being executed on, and that seems to be the true problem.

Fixed by #219.