Bug 18408 - Leak when using AddChildViewController/RemoveFromParentViewController
Summary: Leak when using AddChildViewController/RemoveFromParentViewController
Status: RESOLVED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: Xamarin.iOS.dll (show other bugs)
Version: 7.2.0
Hardware: PC Mac OS
: Normal normal
Target Milestone: Untriaged
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2014-03-14 18:17 UTC by Adam Kemp
Modified: 2014-04-10 11:18 UTC (History)
4 users (show)

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


Attachments
Example project (8.99 KB, application/zip)
2014-03-14 18:17 UTC, Adam Kemp
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 Adam Kemp 2014-03-14 18:17:57 UTC
Created attachment 6321 [details]
Example project

There is a bug in Xamarin's bindings for UIViewController that causes child view controllers to be leaked (i.e., references are held to objects that should be allowed to be GCed). To reproduce the issue you just need to add a view controller as a child of another and then remove it. Run the attached project. Notice that there is a Console.WriteLine in the finalizer for the child view controller, but that code never runs. The two ways of working around the issue are shown as commented out lines in the test code:

         var childViewController = new LeakTestViewController();
         viewController.AddChildViewController(childViewController);
         childViewController.RemoveFromParentViewController();
         // Uncommenting either of these lines will fix the leak.
//         var childViewControllers = viewController.ChildViewControllers;
//         viewController.AddChildViewController(new LeakTestViewController());

The problem is caused by the private __mt_ChildViewControllers_var field in UIViewController. I'm not sure why this field exists in the first place, but the problem is that it is only updated by the ChildViewControllers property getter. When you call AddChildViewController it has extra code that calls the property getter and then ignores the output, but there is no code in RemoveFromParentViewController to force the private field of the parent to be updated after the removal.

The UIViewController hierarchy mechanism works very similarly to the UIView hierarchy mechanism. Each view controller has an array of children (ChildViewControllers), and you remove a child from its parent by calling a method on the child (RemoveFromParentViewController). There is code in Xamarin's binding for UIView.RemoveFromSuperview to avoid this leak:

	if (superview != null)
	{
		UIView[] subviews = superview.Subviews;
	}

The equivalent code is missing from UIViewController.RemoveFromParentViewController.
Comment 2 Udham Singh 2014-03-25 06:52:56 UTC
I have checked this issue and able to reproduce it. To reproduce this issue I run the Example project attached with the bug description, debug the code and noticed the same issue mentioned into bug description.

Screencast: http://screencast.com/t/hFQx82qUW

Environment Info:

=== Xamarin Studio ===

Version 4.2.3 (build 60)
Installation UUID: 011d70a5-dede-428b-ab04-ef451c2e539d
Runtime:
	Mono 3.2.6 ((no/9b58377)
	GTK+ 2.24.23 theme: Raleigh
	GTK# (2.12.0.0)
	Package version: 302060000

=== Apple Developer Tools ===

Xcode 5.1 (5035)
Build 5B45j

=== Xamarin.iOS ===

Version: 7.2.0.2 (Enterprise Edition)
Hash: 58c3efa
Branch: 
Build date: 2014-10-03 18:02:26-0400

=== Xamarin.Mac ===

Xamarin.Mac: Not Installed

=== Xamarin.Android ===

Version: 4.12.1 (Enterprise Edition)
Android SDK: /Users/MM/Desktop/android-sdk-macosx
	Supported Android versions:
		2.1   (API level 7)
		2.2   (API level 8)
		2.3   (API level 10)
		3.1   (API level 12)
		3.2   (API level 13)
		4.0   (API level 14)
		4.0.3 (API level 15)
		4.1   (API level 16)
		4.2   (API level 17)
		4.3   (API level 18)
		4.4   (API level 19)
Java SDK: /usr
java version "1.6.0_65"
Java(TM) SE Runtime Environment (build 1.6.0_65-b14-462-11M4609)
Java HotSpot(TM) 64-Bit Server VM (build 20.65-b04-462, mixed mode)

=== Build Information ===

Release ID: 402030060
Git revision: 30c4afc300c2a39ec5300851357ce02e49dd217e
Build date: 2014-03-05 22:09:33+0000
Xamarin addins: f8a9589b57c2bfab2ccd73c880e7ad81e3ecf044

=== Operating System ===

Mac OS X 10.9.2
Darwin MacMini.local 13.1.0 Darwin Kernel Version 13.1.0
    Thu Jan 16 19:40:37 PST 2014
    root:xnu-2422.90.20~2/RELEASE_X86_64 x86_64
Comment 3 Sebastien Pouliot 2014-04-07 14:12:10 UTC
This issue is resolved when using the New-Refcount (NRC) feature [1]. This is not a new feature [2] but it has totally revamped [3] in the last two months based on the bug reports, like this one.

With XI 7.2.1 this features works with both Boehm (default) and Sgen garbage collectors. NRC has also been enhanced and, right now, there are no known issues (bugs) against it.

While NRC is not the default option (in 7.2.1) we plan to make it so in the near future. Additional testing and feedback on the feature would be appreciated.

[1] http://docs.xamarin.com/guides/ios/advanced_topics/newrefcount/
[2] http://docs.xamarin.com/releases/ios/MonoTouch_5/MonoTouch_5.2/
[3] Newer versions of Xamarin Studio will remove the experimental tag on the feature when XI 7.2.1+ is used.
Comment 4 Adam Kemp 2014-04-07 14:21:37 UTC
Is there anything you can share regarding the expected timeline of 7.2.1? We're on a tight schedule, and I'd like to know how it fits with the availability of a stable release of NRC.