infinitered / redpotion

We believe iPhone development should be clean, scalable, and fast with a language that developers not only enjoy, but actively choose. With the advent of Ruby for iPhone development the RubyMotion community has combined and tested the most active and powerful gems into a single package called RedPotion
MIT License
233 stars 40 forks source link

Update documentation to reflect memory-leaking anti-pattern #150

Closed buffpojken closed 8 years ago

buffpojken commented 9 years ago

So, apparently there exists a distinct anti-pattern ( I guess? ) which still somehow feels sort of natural, but which will crash even a simple application due to memory over-consumption. See referenced project for example - below follows findings from a 49-run Instruments party :)

When creating custom views which in turn includes subviews, assigning the returned RMQ-object to an instance-variable (as is the common pattern when not using RMQ/RedPotion and in Obj-C), such as:

class SomeView < UIView

def initWithFrame(rect)
if super
@label = append!(UILabel, :some_style)
end
self
end
end

will create a hard, circular reference which will prevent this view from being deallocated at all. This doesn't create major issues with table views, collection views and similar due to re-use, but crashes for example most kinds of custom display views (I detected the issue using a ZLSwipeableView, but has managed to recreate it using most common patterns), where it's expected that nilling all references will result in deallocation.

Changing @label to label solves the issue, but that's not the standard pattern.

Either the object returned needs to keep weak refs (to the controller, I guess?) instead, or the documentation needs to be updated to clearly state that this is an anti-pattern when using RedPotion, and clearly advocate the:

find(self).children(author_name_style).get.text = shorti.author_name

pattern instead.

I'll be happy to PR either way, but would love some feedback on which way to go.

See https://github.com/buffpojken/append-memory-leak for example displaying the issue.

jamonholmgren commented 9 years ago

Yeah, this is something I've done for a while. I usually use the cleaner .data version:

find(:author_name_style).data(shorti.author_name)
buffpojken commented 9 years ago

Sure thing, that's what I actually use as well.

But I think it would be neat to make this much more explicit in the documentation, since storing references in instance vars is sort of an expected pattern when not using RP?

jamonholmgren commented 9 years ago

I agree!

buffpojken commented 9 years ago

I'll write something up and PR it.