Bug 16026 - "Message sent to a deallocated object (zombie)" when using string instead of NSString in binding
Summary: "Message sent to a deallocated object (zombie)" when using string instead of ...
Status: RESOLVED UPSTREAM
Alias: None
Product: iOS
Classification: Xamarin
Component: Tools ()
Version: 7.0.2.x
Hardware: PC Mac OS
: --- normal
Target Milestone: Untriaged
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2013-11-07 20:46 UTC by Brendan Zagaeski (Xamarin Team, assistant)
Modified: 2013-11-08 13:02 UTC (History)
2 users (show)

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

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 UPSTREAM

Description Brendan Zagaeski (Xamarin Team, assistant) 2013-11-07 20:46:09 UTC
Reported on behalf of a user.

## Steps to reproduce

1. Open the attached example project.
2. Upload the "iOS" app to device from Xamarin Studio.
3. Open the "iOS -> Memory -> Zombies" template in Apple Instruments.
4. In Instruments, select the "iOS" app, and press "Record" to launch it on the device.



## Result

While running the `viewVideo0.Start()` method, the application crashes, and Instruments shows:
"An Objective-C message was sent to a deallocated 'CFString (immutable)' object (zombie) at address: 0x18144990."



Studying the generated bindings in the library project reveals that the CFString in question is indeed released at the end of the "SetupWithService()" method (in ALVideoView2.g.cs):

NSString.ReleaseNative (nssinkId);



The problem is that the Objective-C method invoked by `SetupWithService()` assigns this NSString pointer to the private `_sinkId` property of `viewVideo0`. When `viewVideo0.Start()` later attempts to access this `_sinkId` property, the app crashes.



## Fix and question

This problem can be fixed by changing the ApiDefinition.cs so that all of the `ALVideoView2` methods use NSString directly instead of C# strings.

The question is, is this the correct, intended behavior? For example, is this one of the "pointer comparison" cases mentioned in the documentation [1]?


[1] http://docs.xamarin.com/guides/ios/advanced_topics/api_design/nsstring/#1.exceptions-to-the-rule


Thanks!




## Version information
Xamarin.iOS 7.0.2.7
Tested on iPhone 4, iOS 7
Comment 2 Sebastien Pouliot 2013-11-07 22:25:34 UTC
From your description that sounds like the API copy the `NSString` pointer with*out* taking a reference - i.e. bad thing (tm)! If that's the case then this is an issue in the ObjC library. You simply can't keep pointers in a referenced counted environment.

> The question is, is this the correct, intended behaviour?

Yes. When a `System.String` is used (in an API) it is converted to an `NSString` when needed. This is generally a short-lived instance (for the duration of the call) unless the code being called "retain" on the `NSString`.

When the binding call returns the `NSString` instance is released (the NativeRelease call you spotted). So it's reference count is reduced by one. If it was one (as when we created it) then the instance is freed. If it was two (e.g. if the call called retain on the instance) then it won't be freed (until the library release it again).


Using an NSString in the API definition let you "control" the lifetime of the string (it will live longer, e.g. until it's GC'ed or Dispose'd since NativeRelease won't be called immediately). So it can be used as a workaround but it's NOT a fix - and if not used/fixed correctly can only make it harder to detect the issue. You're effectively (and manually) managing the life of the `NSString`.

> "pointer comparison" cases mentioned in the documentation [1]?

No, that's something else. Also bad, but much less than not retaining your copies of NSString.


Q: do we have the source of that library ? 

If not do we have the header files (to be sure it's bound correctly) ?
Comment 3 Brendan Zagaeski (Xamarin Team, assistant) 2013-11-08 12:23:30 UTC
Thanks for the extra explanation! And indeed you are exactly right. This was a bug in the AddLive library. The user who reported the problem contacted the library developers about this issue. The AddLive team confirmed that there was a problem with the way they were retaining the _sinkId property, and they have now released a corrected version of the library.

This bug can be closed. Thanks!
Comment 4 Sebastien Pouliot 2013-11-08 13:02:06 UTC
np