Correct way to pop a DialogViewController? Mine are staying in memory
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
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
- Abuse
0 • LikeHi 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
- Abuse
0 • LikeOk 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.
- Abuse
0 • LikeFixed 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
- Abuse
0 • LikeActually 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
- Abuse
0 • LikeHi John,
Event handlers hold references in .NET (both the MS implementation and Mono)--they will keep objects alive well past their expiration date in certain scenarios. I'm not quite clear based on your post, but you should try ensuring that you unsubscribe all event handlers to ensure DVC's will be collected.
- Abuse
5 • LikeI 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
- Abuse
0 • Like@HairyJohn Delegates and events can be beasty. Imagine you have a long living object A. That object constructs a short living object B. B exposes an event. Now the long living A attaches itself to this event: B.SomeEvent += A.SomeHandler. Now you bring B on screen (assuming it is some view). At some point your event handler is triggered. That signals A that B should be dismissed. So you remove your view and set B to NULL. And now one would think that B will get collected, right? It won't. Long living A holds a reference through the event handler. Unless you unsubscribe (B.SomeEvent -= A.SomeHandler), B cannot be collected. This is not Mono or MonoTouch but .NET.
- Abuse
5 • LikeIn 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
- Abuse
0 • LikeI saw some odd behavior at one point in the SQLbits conference code - but I assumed it was my bug...
Basically:
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!
- Abuse
0 • LikeThere is a problem with cyclic dependencies and events which usually don't happen with pure .NET programs, because MonoTouch has to co-exist with Objective C's refcounting.
For all instances of types derived from NSObject there are actually two instances, a native object and a corresponding managed wrapper for that object. The problem arises because MonoTouch has to obey one rule for types you have derived from types in monotouch.dll (such as UIView) - the managed wrapper cannot be freed before the corresponding native object. Technically this means that the managed wrapper will stay alive at least until the native refcount is exactly 1 (because the managed wrapper itself refs the native object, it now knows that the only ref keeping the native object alive is the one it holds itself).
Going back to your cycle, this is what happens:
As you see the custom drawn UIView is not eligible for garbage collection as long as it is stored in the AccessoryView property, the parent element is referenced by its parent DVC, the parent DVC is referenced by the event...
- Abuse
7 • 2Like@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!)
- Abuse
0 • LikeHi 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 :)
- Abuse
0 • LikeRolf, 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?
- Abuse
0 • LikeThere 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.
- Abuse
0 • Like@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?
- Abuse
0 • Like@Krumelur
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).
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).
There is one trick you may want to try: set the Delegate property to null.
This will detach from all ObjectiveC events in at once (it will not work for managed-only events in your own code).
- Abuse
3 • 3Like