tgrapperon / swift-dependencies-additions

More dependencies for `swift-dependencies`
MIT License
298 stars 39 forks source link

Avoid deadlock when initializing default value of PersistentContainer #46

Closed KaiOelfke closed 1 year ago

KaiOelfke commented 1 year ago

Sorry for my late reply. I was busy before. Your fix almost works. But the problem is that the live and preview value use the default static func, which still accesses viewContext during initialization. Removing this line fixes it and the configuration should still happen later in the closure then.

tgrapperon commented 1 year ago

Hey @KaiOelfke! Thanks for the PR. Removing this flag bothers me a little because this is the default from Apple's template, and likely what users would expect in a general manner. There are likely alternative workarounds. Do you have a simple repro of the issue?

KaiOelfke commented 1 year ago

Hey @KaiOelfke! Thanks for the PR. Removing this flag bothers me a little because this is the default from Apple's template, and likely what users would expect in a general manner. There are likely alternative workarounds. Do you have a simple repro of the issue?

I can try to create one. I need this flag also in my app, but it’s still applied via the closure you made at first access. This line only removes the early configuration for the default(inMemory:) function. It’s not removing the configuration in the closure.

KaiOelfke commented 1 year ago

Probably, there's better ways to make a simple sample. I don't use OperationQueue's in my project. But I wanted to make something without TCA.

In my project the main thread is running printer reducer code for debug printing and the main thread can't acquire the dependency values lock to resolve some printing dependency. The dependency values lock is hold by the persistent container default value function default(inMemory:), but the background thread running it waits for access to the main thread to set the flag on the view context.

import SwiftUI
import DependenciesAdditions

struct ContentView: View {
    var body: some View {
        VStack {
            Image(systemName: "globe")
                .imageScale(.large)
                .foregroundColor(.accentColor)
            Text("Hello, world!")
        }
        .padding()
        .onAppear {
          let op = BlockOperation {
            print("start")
            @Dependency(\.persistentContainer) var persistentContainer
            print("done", persistentContainer.newBackgroundContext())
          }
          let queue = OperationQueue()
          queue.addOperation(op)
          queue.waitUntilAllOperationsAreFinished()
          @Dependency(\.persistentContainer) var persistentContainer
          print("flag", persistentContainer.viewContext.automaticallyMergesChangesFromParent)
        }
    }
}

Currently the liveValue and previewValue of PersistentContainer use public static funcdefault(inMemory: Bool = false) -> PersistentContainer. This function implicitly requires access to the main thread to return for setting the view context flag. It will block until it can run on the main thread.

Dependencies resolves and caches dependency values at first access according to my knowledge. The dependency may be first accessed on a non main thread. In this case it's first resolved and initialized inside this BlockOperation running on a non main OperationQueue. But the main thread is already running and blocked by waiting for the queue to finish all operations. So the PersistentContainer dependency can't finish initializing. And that's the deadlock.

With the changes you made in this branch with this closure

let viewContext = { @Sendable in
        let context = persistentContainer.viewContext
        isViewContextConfigured.withValue { isConfigured in
          if !isConfigured {
            context.automaticallyMergesChangesFromParent = true
            isConfigured = true
          }
        }
        return context
      }

and my change that removes persistentContainer.viewContext.automaticallyMergesChangesFromParent = true from the default(inMemory:) function, but not your closure the deadlock is gone and the flag is applied as expected.

Console output:

start
done <NSManagedObjectContext: 0x600003c10680>
flag true
tgrapperon commented 1 year ago

Ah yeah, sorry, I thought your PR was targeting main, not cd-dl! I indeed forgot to remove the problematic line after implementing the lazy workaround. Sorry for not getting it sooner. Of course, no problems with the change.