Bug 19728 - SGen + Reference counting extension causes crash
Summary: SGen + Reference counting extension causes crash
Status: RESOLVED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: XI runtime ()
Version: 7.2.2
Hardware: Macintosh Mac OS
: --- normal
Target Milestone: Untriaged
Assignee: Sebastien Pouliot
URL:
Depends on:
Blocks:
 
Reported: 2014-05-13 10:41 UTC by Neal
Modified: 2014-05-21 17:48 UTC (History)
3 users (show)

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


Attachments
demo project (12.48 KB, application/zip)
2014-05-13 10:42 UTC, Neal
Details
updated demo (19.58 KB, application/zip)
2014-05-14 21:51 UTC, Neal
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 Neal 2014-05-13 10:41:30 UTC
Hello,

I have a reproducable test case for you that causes a crash when the Reference counting extension is used with SGen.  I'm using MT 7.2.2.2 and XamStudio 4.2.5 - all stable as of today.

Run that attached sample project in an iOS 7.1 iPhone 4 simulator (not iPad).  With SGen and Reference counting extension checked run the app, tap the button to show the view, tap Cancel on the upper left.  Repeat this and it will crash.

Now turn off the reference counting extension but leave SGen checked and repeat the above - no crash.

FYI

Neal Culiner
NC Software, Inc.
Comment 1 Neal 2014-05-13 10:42:10 UTC
Created attachment 6792 [details]
demo project
Comment 2 Sebastien Pouliot 2014-05-13 11:57:21 UTC
That's normal (i.e. not a bug).

This happens because you are calling `AddObserver` 
* without a target; and 
* never calling `RemoveObserver`. 

In effect the notifications are called on every `MonkeyViewController`. IOW the 2nd time you tap it's called on the first and 2nd VC (why the first ? something stills holds a reference to it and it's alive in the native side).

Now the NavigationController for the older (first) VC is not available anymore (so you get a `null` and a NullReferenceException).


You can solve this by either:

a) specifying which object instance will receive the notification, e.g.

	NSNotificationCenter.DefaultCenter.AddObserver (UIKeyboard.WillShowNotification, KeyboardShown, this);
	NSNotificationCenter.DefaultCenter.AddObserver(UIKeyboard.WillHideNotification, KeyboardHidden, this);

b) calling RemoveObserver (for both) when you close you VC (e.g. pass a delegate to PresentViewController completionHandler instead of `null`);


This does not happen without NRC because this is part of what NRC solves - i.e. it won't artificially keep the NavigationController alive (in a backing field) since it's now being referenced anymore.
Comment 3 Neal 2014-05-13 12:01:18 UTC
Thank you.
Comment 4 Neal 2014-05-14 21:51:32 UTC
Sebastien,

My demo was incomplete.  I checked my code in my app and saw that I was in fact calling removeObserver.  I updated the demo.  Can you check this out and see if this is a valid bug or something incorrect in my code again.

Thank you.

Neal

Attaching demo-2
Comment 5 Neal 2014-05-14 21:51:50 UTC
Created attachment 6813 [details]
updated demo
Comment 6 Sebastien Pouliot 2014-05-14 21:54:02 UTC
Strange, it did work fine for me using RemoveObserver. I'll have a look ASAP.
Comment 7 Sebastien Pouliot 2014-05-14 22:22:24 UTC
So Apple notification API is not that cute (I wish it was).

You need to remove the notification based on the NSObject returned from AddObserver, e.g.

		NSObject shown;
		NSObject hidden;

        public override void ViewDidLoad()
        {
            base.ViewDidLoad();
            cancelButton.Clicked += delegate
            {
                DismissViewController(true, null);
            };

            if (!IsPad)
            {
				shown = NSNotificationCenter.DefaultCenter.AddObserver(UIKeyboard.WillShowNotification, KeyboardShown);
				hidden = NSNotificationCenter.DefaultCenter.AddObserver(UIKeyboard.WillHideNotification, KeyboardHidden);
            }

            textField.BecomeFirstResponder();
        }

        public override void ViewWillDisappear(bool animated)
        {
            base.ViewWillDisappear(animated);
            if (!IsPad)
            {
				NSNotificationCenter.DefaultCenter.RemoveObserver(shown);
				NSNotificationCenter.DefaultCenter.RemoveObserver(hidden);
            }
        }


Your sample works fine like this. It's how I originally did it (but I did not keep it that way, I modified it to use option (a) where I gave `this` to a different AddObserver overload).
Comment 8 Neal 2014-05-14 22:49:00 UTC
Ok - thanks.  Will get my code cleaned up.