square / Cleanse

Lightweight Swift Dependency Injection Framework
Other
1.78k stars 90 forks source link

Fix WeakProviders becoming deinitialized. #131

Closed sebastianv1 closed 4 years ago

sebastianv1 commented 4 years ago

Upon a Graph instance's finalize function, we clear out the dictionary of futureProviders. The dictionary holds all instances of FutureProviders that represent a type whose factory instance may not have been created yet. When we create the correct Provider instance inside our graph, it retains a strong reference to the FutureProvider instance, so even though we clear out the dictionary after finalize, these instances are still in memory as they are being retained by their equivalent Provider instances.

This poses an issue for WeakProviders as their wrapped Provider instance rightfully holds onto a weak instance of the FutureProvider. Therefore, if no other Provider retains its equivalent FutureProvider, it will be deinitialized and all WeakProviders for a type will return nil.

This one line change deletes the call to clear out the dictionary of FutureProviders after finalize. The existing implementation only allows us to build a graph once and will throw an error if we try to mutate a graph after construction. This is validated using the finalized boolean stored internally. Subcomponents all construct and finalize their own Graph instances, and don't reuse existing futureProvider dictionaries. Thus, it doesn't seem that keeping the objects stored inside the dictionary will cause any functional changes, and we already know that we won't be causing any extra memory overhead as existing functionality today already retains all FutureProvider instances.

sebastianv1 commented 4 years ago

Not sure why travis-ci isn't running. Looking now