Bug 3189 - UIViewController.PresentModalViewController() holds obsolete reference to presented controller
Summary: UIViewController.PresentModalViewController() holds obsolete reference to pre...
Status: RESOLVED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: XI runtime ()
Version: 5.0
Hardware: Macintosh Mac OS
: --- normal
Target Milestone: Untriaged
Assignee: Sebastien Pouliot
URL:
Depends on:
Blocks:
 
Reported: 2012-02-02 05:04 UTC by René Ruppert
Modified: 2012-02-26 12:12 UTC (History)
4 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
Screenie of memory profiler (20.36 KB, image/png)
2012-02-02 05:04 UTC, René Ruppert
Details
Demo of the issue (7.35 KB, application/zip)
2012-02-02 05:05 UTC, René Ruppert
Details
hot fix (3.02 MB, application/octet-stream)
2012-02-09 09:09 UTC, Sebastien Pouliot
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description René Ruppert 2012-02-02 05:04:47 UTC
Created attachment 1303 [details]
Screenie of memory profiler

Monotouch 5.2.1
Mono 2.10.8.1
MonoDevelop 2.8.6.4

Issue:
- Present a controller B using PresentModalViewController() from controller A
- Dismiss the modally presented controller B
- Controller A will hold a reference to B that will not get collected
- In addition a mysterious type System.Eventhandler also a reference to B

Reproduce:
Run the attached demo solution in profile mode.
Make snapshot after the app has started.
Hit the "Present" button.
Make another snapshot.
Hit the dismiss button.
Make a third snapshot.

Compare the snapshots. Snapshot 1 has no references to ModalController.
Snapshot 2 obviously has a reference as the controller is currently being presented.
Snapshot 3 still has the reference although the controller has already been dismissed.

Attachments:
- Demo project is attached.
- Screenshot of memory profiler was made after the modally presented controller has already been dismissed and shows that RootController hold a ref to ModalController.
Comment 1 René Ruppert 2012-02-02 05:05:06 UTC
Created attachment 1304 [details]
Demo of the issue
Comment 3 Sebastien Pouliot 2012-02-02 15:27:11 UTC
Thanks for the small test case! 

It does look identical to the other issue I was looking. A reference is kept in the ModalViewController property and the *Unload overrides are never called.
Comment 4 Sebastien Pouliot 2012-02-02 21:54:34 UTC
I got a local fix. It looks like ModalViewController does not return `null` (anymore? *) after a call to `DismissModalViewControllerAnimated` so we keep our (backing field) reference to the controller and that prevented the GC to collect the object and so on...

* I need to check how (if?) this got broken too (maybe an iOS update changed the behavior) and do some more testing but, again, thanks for providing the small test case.

note: it's likely just the test case (and not your app) but creating things in ViewWillAppear also made memory grow (one more button each time) so it should be initialized earlier.
Comment 5 René Ruppert 2012-02-03 03:28:02 UTC
I have the same problem in my real app, not only in the test case. In FinishedLaunching() I immediately call PresentModalViewController() to overlay the whole screen with a PIN dialog.

I don't understand your "note". Are you telling me I should not create things in ViewWillAppear()?
Comment 6 Sebastien Pouliot 2012-02-03 07:57:27 UTC
About the note, add:

			Console.WriteLine ("RootController::ViewWillAppear {0}", View.Subviews.Length);

in your test case RootController's ViewWillAppear method, then click "Present" and "Close" several times and look at the "Application Output" pad.

Moving the code earlier (e.g. in ViewDidLoad) or adding a "setup" flag in ViewWillAppear can ensure only one button will be created.
Comment 7 René Ruppert 2012-02-03 08:26:32 UTC
Yeah, get it. You're right. That's just an issue of the test project. I didn't bother.
Comment 8 Sebastien Pouliot 2012-02-07 10:15:26 UTC
A fix has been pushed to master (for the next major version of MonoTouch). It needs a bit of testing before it can be backported into the (soon to be released) 5.2.x stable branch.

Let me know if you wish to test this and I'll attach an hotfix+instructions to the bug report.

Again thanks for your test case.
Comment 9 René Ruppert 2012-02-07 10:18:41 UTC
Does it fix the modal issue explicitely or also other cases that kept references?
If it also fixes other cases, I'd be happy to give it a try.
Note that I'm only assuming that there are other problems, I have no proof. I'm just seeing references here in the memory monitor that I didn't expect but I haven't investigated any further.
Comment 10 Sebastien Pouliot 2012-02-07 10:30:52 UTC
The fix is only for [Present|Dismiss]ModalViewController... 
but it (now) cover cases where a hierarchy is used (and dismissed anywhere within it).

This same fix does not apply elsewhere. The main issue here was that the ModalViewController property was _assumed_ to return null after DismissModal* was called (and that's not the case, at least not quick enough, so the reference was never null'ed out).

Let us know if you find anything else! Thanks
Comment 11 René Ruppert 2012-02-07 10:34:17 UTC
If you can, please send me a DLL to test with; the reason is that only then I can see if the other references will also go away. If not, I'll investigate and report other issues if I find something.
Thanks for fixing it so quickly!
Comment 12 Sebastien Pouliot 2012-02-09 09:09:08 UTC
Created attachment 1331 [details]
hot fix

To use the attached assembly (on top of MonoTouch 5.2.3) please do the following steps:

1) backup your /Developer/MonoTouch/usr/lib/mono/2.1/monotouch.dll and /Developer/MonoTouch/usr/lib/mono/2.1/monotouch.dll.mdb files

2) copy the attached file to /Developer/MonoTouch/usr/lib/mono/2.1/monotouch.dll

3) remove the /Developer/MonoTouch/usr/lib/mono/2.1/monotouch.dll.mdb symbols (they won't match anymore)

4) clean, rebuild and test your application
Comment 13 Sebastien Pouliot 2012-02-16 15:22:05 UTC
A regression was found (see bug #3489) so the fix will be removed from stable (in 5.2.5) until a better fix (and more testing) can be done on this issue. 

Note: you can keep using the attached assembly on top of MT 5.2.5 if you need the fix (and don't suffer from it's drawback).
Comment 14 Sebastien Pouliot 2012-02-26 12:12:23 UTC
A fixed-fix is now in 5.2-series and will be available in 5.2.6