luispadron / UIEmptyState

An empty state control to give visually appealing context when building iOS applications.
MIT License
178 stars 34 forks source link

Remove retain cycles on dataSource and delegate properties #9

Closed piotrzuzel closed 7 years ago

piotrzuzel commented 7 years ago

Changed objc_AssociationPolicy in emptyStateDataSource and emptyStateDelegate associatedObject setters to .OBJC_ASSOCIATION_ASSIGN from .OBJC_ASSOCIATION_RETAIN.

This will ensure that UIEmptyStateDataSource and UIEmptyStateDataSource implementations are stored as weak references and no longer cause retain cycles in UIViewControllers using default delegate and data source implementations.

I am opening another pull request because last one contained unnecessary change and also I done goofed with updating previous pull request ¯\_(ツ)_/¯

luispadron commented 7 years ago

Wow can't believe I missed this.

I'm not an objective-c expert but shouldn't these objects be weak? or is assign the same thing?

Thanks for the pull request! I'm going to look over it more and test it a bit after lunch, and then get this merged in.

piotrzuzel commented 7 years ago

According to objc_AssociationPolicy documentation:

OBJC_ASSOCIATION_ASSIGN Specifies a weak reference to the associated object.

Unfortunately there is no OBJC_ASSOCIATION_WEAK policy therefore I used OBJC_ASSOCIATION_ASSIGN which acts as unsafe_uretained ownership qualifier. This works pretty similar to regular weak qualifier - they both hold a weak reference to an object. The main difference between them is that weak qualifier will convert to nil when property becames nil and is being accessed. On the other hand unsafe_unretaied qualifier continues pointing to the same address and may lead to crashes due to accessing deallocated object.

piotrzuzel commented 7 years ago

In the meantime I also am performing more tests to this solution.

luispadron commented 7 years ago

Maybe something like this would also be nice to have as it would be better if the object was nil, this requires a bit more work but I can do it if you don’t have time.

piotrzuzel commented 7 years ago

This seems like a good compromise. Unfortunately there is no way to handle EXC_BAD_ACCESS runtime exception while accessing pointer to deallocated object.

With my current solution this situation may happen when delegate object other than self will deallocate.

I am now working on implementing solution you have provided above.

luispadron commented 7 years ago

Now available in 2.0.2, thanks for all the work!

aashishtamsya commented 6 years ago

@luispadron The following commits doesn't reflect in the current release Version 4.0.0