ivlevAstef / DITranquillity

Dependency injection for iOS (Swift)
MIT License
421 stars 32 forks source link

Weak parent #154

Closed sssbohdan closed 4 years ago

sssbohdan commented 4 years ago

Hi. In my current architecture, I'm using coordinators with injected containers. (I didn't found method collaborate so I decide to use init(: parent)) Code:

protocol Coordinator: class {
    var parent: Coordinator? { get set }
    var childCoordinators: [Coordinator] { get set }
    var navigationController: UINavigationController { get }
    var container: DIContainer { get set }

    func start()
}

extension Coordinator {
    func store(coordinator: Coordinator) {
        coordinator.parent = self
        childCoordinators.append(coordinator)
        coordinator.container = DIContainer(parent: container)
        coordinator.start()
    }

Today I've checked memory graph and spotted multiple leaks in coordinators. The reason was that your container holding a strong reference on the parent. I've fixed the problem by making container DIContainer! and explicitly assign him nil. Code:

func free(coordinator: Coordinator) {
        coordinator.container.clean()
        coordinator.container = nil
        childCoordinators.removeAll(where: { $0 === coordinator })
    }

But in my opinion, the best solution will be adding weak to parent. Or at least make parent public property. What do you think on this? Maybe I'm using container in the wrong way. Thanks.

ivlevAstef commented 4 years ago

Hi. parent container can contains only by strong reference. But I don't know why you use child contains? Their use is a specific case.

Maybe you need use custom scopes? Or only standard injections without specific functional? I need more information, and see DI code to give a detailed answer.

sssbohdan commented 4 years ago

@ivlevAstef I don't want to register all dependencies right away. So I register dependencies that are required for some screen just before showing it. In my main coordinator, I have general dependencies which I pass to child coordinators by collaborating containers usinginit(parent: ). Edit 1: I still don't understand why you need strong reference on parent container if someone else will keep strong ref on it? Thanks.

ivlevAstef commented 4 years ago

But why? Dependencies Cannot changed until running iOS/macOS application. But if your use dlopen and dynamic libraries then dependencies can changed. If your dependencies not constant on one app run... maybe your register not dependencies in DI? Or dlopen.

About edit1: because DIContainer at ideal need only use for register, and call once resolve. And parent container I'm looked at Swinject - this DI library use strong reference on parent. I don't use it myself.

And other DIContainer (dagger, Weaver, needle, ...) technically Cannon have parent containers, and cannot register other dependencies after run application, but they are used, and dagger2 on Android monopolist.

sssbohdan commented 4 years ago

@ivlevAstef Yes, I see that Swingject doesn't use weak ref on the parent. I moved the project to Swinject and all my random crashes and leaks are gone. I'm not trying to say that your framework is worse or somehow offend you, just saying that Swinject suits for my programming style more :) Thanks :)