infinitered / rmq

RMQ - RubyMotionQuery
MIT License
307 stars 52 forks source link

Event callbacks are retained, preventing deallocation of view controllers #248

Closed dmarkow closed 9 years ago

dmarkow commented 9 years ago

Event callbacks were contributing to major memory issues in our app because the callback blocks were being retained after the view controller was removed.

In our app, we have a controller that deals with raw AVCaptureSession output resulting in approximately 30MB per image capture sitting in memory. Once the user saves the image, the view controller pops back to the list of photos. In the example below, @thing should automatically be released by ARC when the view is unloaded. However, because the .on() event handler is retaining the block, the instance of FooScreen is never removed from memory (the NSLog line under dealloc never executes). So after the user takes around 15-20 photos, a low memory warning is triggered and soon after the app crashes. Instruments confirms the app's memory usage growing by approximately 30mb each time the view appears.

class FooScreen < PM::Screen
  title "Your title here"
  stylesheet FooScreenStylesheet

  def on_load
    append(UIButton, :my_button).on(:touch) { NSLog "touched" }
    @thing = HUGE_STREAM_OF_DATA_THAT_ALLOCATES_30MB
  end

  def dealloc
    NSLog "Deallocing"
    super
  end
end

If I change the callback to be .weak! instead, everything deallocates properly (my NSLog statement executes) and the memory stays relatively flat. For now, I changed all of my callbacks as follows:

append(UIButton, :my_button).on(:touch, &lambda {  NSLog "touched" }.weak!)

Should we consider just making callback blocks weak whenever possible? Sugarcube's events library (which adds similar .on() { ... } functionality) does something similar:

@callback = callback.respond_to?('weak!') ? callback.weak! : callback
twerth commented 9 years ago

Thanks Dylan, we weren't aware of this; ack! Let me poke around a bit. But yeah, making it weak makes sense.

twerth commented 9 years ago

I verified this is true. I thought we tested this in the past.

Removing events will also fix this: rmq.all.off before you close the controller.

Automatically making the block weak has issues, I'm currently working on this.

twerth commented 9 years ago

Thanks again Dylan. Your solution worked. I was able to create a test that showed if failing, then I fixed it exactly as you suggested. The fix is in [master] now, and will be in the next release.

dmarkow commented 9 years ago

Awesome, thank you!