Correct way to pop a DialogViewController? Mine are staying in memory

JohnHairJohnHair John HairGBMember ✭✭
edited October 2012 in iOS

Hi guys, Im profiling my app and have found that my DialogViewController instances are not being collected. Ive tried forcing a gc with GC.Collect(0) but the view controller, its root and all the elements I add on each iteration are staying in memory and overtime this is building up. Im wrapping this up in a UINavigationController and use PresentModalViewController from my core controller, and dismissing with DismissModalViewControllerAnimated. I dont have any references to the DVC in my code, other than a single instance which gets set to null and replaced with a new DLC every time.

Maybe Im not supposed to be creating a new DialogViewController every time? Are they supposed to be reused?

Thanks John

Posts

  • StuartLodgeStuartLodge Stuart Lodge USBeta ✭✭✭
    edited October 2012

    Hi John

    I think what you might be seeing here is the difference between iOS/UIKit memory management and MonoTouch/.Net memory management.

    If you signal iOS to clear up memory - e.g. by using the Hardware > SimulateMemoryWarning menu option - then hopefully that'll get iOS to clear up the unused UIKit controllers - after which Mono will do likewise with the .Net wrappers and inherited classes too.

    If that works... then hopefully somebody with a better lower level understanding than me will be along to explain the details :)

    Stuart

  • JohnHairJohnHair John Hair GBMember ✭✭

    Hi Stuart, Thank you for the suggestion, this is getting even stranger.

    If I use a standard DialogViewController and create, present, dismiss a new instance a few times then take a snapshot, I see I have created x number. If I then run Hardware | Simulate Memory Warning and take another snapshot, the instance count is doubled.

    If I inherit from DialogViewController (call it MyDialogViewController) and create present and dismiss a single instance, then simulate a memory warning, my app crashes with:

    Message: Selector invoked from objective-c on a managed object of type myCal.MyDialogViewController (0xC0F40D0) that has been GC'ed Stack trace: at MonoTouch.ObjCRuntime.Runtime.ConstructNSObject (IntPtr ptr, IntPtr klass) [0x00045] in /Developer/MonoTouch/Source/monotouch/src/ObjCRuntime/Runtime.cs:215 etc...

    I have no code in my receive memory warning except to log the warning itself. I also have no code in my MyDialogViewController class except to override WillRotate.

    Cheers John

  • JohnHairJohnHair John Hair GBMember ✭✭

    Ok I found that I needed to override the handle constructor of my DVC as that gets called when the app receives a memory warning.

    But the instances of my DVCs are still not being released, and in fact the instance count is doubled as MonoTouch/iOS tries to recreate them, although with no content.

  • JohnHairJohnHair John Hair GBMember ✭✭

    Fixed it, turns out the problem was I was keeping a reference to a custom element inside a button I was putting into the custom element's accessory view. My DVC's are now being collected!! Woohoo!! :)

    Cheers John

  • JohnHairJohnHair John Hair GBMember ✭✭

    Actually wasn't just the reference to the element within the accessory view. I also had a TouchUpInside event attached that referenced the DVC. So actually once the DVC instance was replaced it all should have been orphaned and thus available for collection. Im guessing the MT GC isn't quite clever enough to work that out?

    Cheers John

  • JohnHairJohnHair John Hair GBMember ✭✭

    I have converted all references to my DVC inside the event handlers to static, this allows all older instances to be collected nicely. I could unwire the event handlers as you suggest, but I don't think I should have to, in pure .NET this would have been collected methinks.

    Cheers John

  • JohnHairJohnHair John Hair GBMember ✭✭

    In my example my DVC is a short lived object containing another short lived object. Once I dismiss the DVC it, and everything it contains, should be collectable. So to clarify:

    My DVC has a reference to an element. That element has a reference to a custom draw UIView (stored in AccessoryView). That custom draw UIVIew has a reference to a UIButton. That button has a tapped event containing a reference to the DVC.

    Once I dismiss the DVC there are no references to it except from objects it contains and nothing else has references to those either. Only circlical (I know its not a word) references remain, the DVC is thus orphaned.

    I know events add to the complexity, but in my simple mind a reference is a reference is a reference. I thought in .NET the above example would be collected. I'll have a play when I get to work later.

    Cheers! John

  • StuartLodgeStuartLodge Stuart Lodge USBeta ✭✭✭

    I saw some odd behavior at one point in the SQLbits conference code - but I assumed it was my bug...

    Basically:

    • the ViewModels expose some MvxRelayCommands
    • those Commands used anonymous Actions, each of which referenced the ViewModel
    • during UI setup, the Views (ViewControllers) got references to these Commands and bound them to the events exposed by the UI

    With this setup I expected that the only references were from Views -> Commands -> Anonymous Actions -> ViewModels

    So I expected that the Views and ViewModels should be collected together.

    However... actually it seemed like the ViewModels never got collected unless I manually set the ViewModels to Dispose the relay commands, which forced the commands to null there references to the anonymous actions.

    So that was the fix I added - but I never really understood why this fix was needed.

    More background on this:

    I admit this problem was a bit involved - that's why I brushed it aside as my bug at the time... but I will revisit it one day - it's always good to understand these things!

  • StuartLodgeStuartLodge Stuart Lodge USBeta ✭✭✭

    @rolf that does sound quite scary in terms of subtle bugs creeping in! Is there any way at all of hinting to the garbage collection that the circle is free to disappear? In my case, the same code seemed to work fine in WP7 and Droid - it was just in Touch that the circle of references (some in native, some in managed) seemed to keep everything alive.

    I'll need to have a play to see if I can work out what happened again (in a simpler setup than last time!)

  • deapsquatterdeapsquatter Kevin Knoop USMember, Beta

    Hi all,

    I went through the same issue and eventually filed a bug report which is where I got informed on the above information.

    It definitely does require a level of understanding above what you would need to know using regular mono/.net runtimes. I actually find it difficult to keep this problem at bay while coding as my mind is more in a functional state. What I do afterwards is to profile the app and then look for these type of issues. Profiling before release is good practice anyway so I don't have a problem spending some time doing this.

    I think I echo the sediments of others when I suggest some sort of "things to look out for" document outlining potential gotcha's which will manage garbage collection expectations.

    Still the best product I have used and this is a relatively small price to pay. I'm sure memory management will just better as time goes on. For now, lets all just get informed :)

  • deapsquatterdeapsquatter Kevin Knoop USMember, Beta
    edited October 2012

    Rolf, The scenario above is a fairly standard way I think of working with cells and custom views. How would you have handled the scenario above differently to make sure everything is GC'd? Can you set the AccessoryView to null at some point? When would be a good time to do that? Also does UITableViewCell.ContentView.Add(UIView) also inc the ref count on the UIView?

  • RolfBjarneKvingeRolfBjarneKvinge Rolf Bjarne Kvinge USModerator, Xamarin Team Xamurai

    There are a couple of ways of breaking the cycle:

    • Detach the event handlers when the DVC and its children are removed from the view hierarchy. Exactly when depends a bit on each app, but usually the ViewDidDisappear notification is a good fit.

    • Null out the AccessoryView - this may be a bit harder to identify since it's not only the AccessoryView property, but many properties.

  • BrainloopBrainloopBrainloopBrainloop Brainloop Brainloop DEMember, Beta ✭✭

    @rolf Detaching the event handler is always a good idea but that leads to two other questions. 1. Often, event handlers are coded using inline delegates or lambdas. These cannot be removed using the -= operator. Does this mean, using inlines in MonoTouch is not recommended? (Hard to believe) 2. Or does using inline delegates/lambdas instead of "real" methods prevent the problem in the first place?

  • RolfBjarneKvingeRolfBjarneKvinge Rolf Bjarne Kvinge USModerator, Xamarin Team Xamurai

    @Krumelur

    1. We don't recommend one way or another when using events, use the way you prefer. That said, if you need to remove event handlers, it's tricky to use lambda event handlers (you can store the lambda in a variable though - but it's obviously not as convenient as just the lambda).

    2. Inline delegates / lambdas do not prevent the problems in the first place. In fact, they tend to aggravate it a little bit (since in many cases they will implicitly add an additional reference to 'this', which tend to trip people who don't know very well how lambdas actually work).

    3. There is one trick you may want to try: set the Delegate property to null.

      obj.Delegate = null;
      

      This will detach from all ObjectiveC events in at once (it will not work for managed-only events in your own code).

Sign In or Register to comment.