uber/needle

Proper usage of shared to avoid memory cycles

aurelienp opened this issue · 5 comments

I struggle to wrap my head around memory management in Needle and I'm looking for guidance, or maybe just confirmation that I understand things well.

Non-Pluginized Components

class ParentComponent: Component<EmptyDependency> {
    var viewController: ParentVC {
        shared {
            ParentVC(childVCBuilder: childComponent)
        }
    }

    var childComponent: ChildComponent {
        ChildComponent(parent: self)
    }
}

class ParentVC: UIViewController {
    let childVCBuilder: ChildVCBuilding

    init(childVCBuilder: ChildVCBuilding) {
        self.childVCBuilder = childVCBuilder
    }
}

protocol ChildVCBuilding {
    var viewController: ChildVC { get }
}

class ChildComponent: Component<EmptyDependency>, ChildVCBuilding {
    var viewController: ChildVC {
        ChildVC()
    }
}

I expect this code to create a memory cycle (ParentComponent -> ParentVC -> ChildComponent -> ParentComponent), hence my first question: is it correct to say that a component that has children components should not use shared when creating their associated VC?

Pluginized Components

I actually have less problems with pluginized components. I understand that the plugin extension weakly (with unowned) reference the non-core component so the only potential cycle is if a developer doesn't properly bind the pluginized component to the VC.

Though, I guess it doesn't make much sense to bind a component with multiple VC, hence my second question: is it correct to say that pluginized components must always used shared when creating their associated VC?

Combining the two

Combining the two first observations, assuming they are correct, is it correct to say that a PluginizedComponent should not have children components (apart from its non-core component), otherwise we're stuck in a contradiction where the VC both must be wrapped in shared (as a pluginized component) and not wrapped in shared (as a component with children components).

I guess some follow-up questions would be: what problem do PluginizedComponents solve?

It seems that it's there to add some structure, but I'm struggling to see whether this structure solve any problem in itself, or if it's there to power a specific plugin system that you built on top of Needle?

rudro commented

First, the Pluginized stuff is, to the best of my knowledge, only used by Uber. We have this complex scheme of having "core" and "non-core" features with some restricted ways in which the core can interact with the non-core components. At this time, we hope that everyone outside of Uber just ignores all the contents of the pluginized directories in the source.

Now, regarding memory cycles: The most important item to be mindful of is the last link in your example. Child components (by design) always hold on to their parents strongly. Since there are these "upward" arrows, it's very important that we don't create any downward ones (well, ones that involve components). So, in your example the ParentVC -> ChildComponent is not ok. At Uber, we don't pass an instance of the child component to the parentVC. Rather, we have a closure passed in, which -- when run -- creates an instance of the component. ParentVC(childVCBuilder: { return self.childComponent}) instead. In our case, it's possible to have multiple instances of the same childVC type, so we run this closure each time a new child needs to be created.

So, in your example the ParentVC -> ChildComponent is not ok. At Uber, we don't pass an instance of the child component to the parentVC. Rather, we have a closure passed in, which -- when run -- creates an instance of the component. ParentVC(childVCBuilder: { return self.childComponent}) instead.

Doesn't this still result in a cycle? Given that the closure retains self (ParentComponent)?
ParentComponent -> shared -> ParentVC -> closure -> ParentComponent (self)

Or is this OK because the idea is that ParentComponent ends up being transitory?

@GyroJoe I think you are right.
If we want to avoid the cycle, ParentComponent cannot retain ChildComponent in any ways.
That means we cannot use shared to store the ChildComponent.

Thanks @haifengkao for confirming. I'll mark this as resolved then.