This is Xamarin's bug tracking system. For product support, please use the support links listed in your Xamarin Account.
Bug 1086 - [Discussion]: How to correctly handle copyWithZone: ?
Summary: [Discussion]: How to correctly handle copyWithZone: ?
Status: RESOLVED FIXED
Alias: None
Product: MonoMac
Classification: Desktop
Component: Bindings (show other bugs)
Version: GIT
Hardware: PC Mac OS
: --- normal
Target Milestone: ---
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2011-09-27 16:32 UTC by Curtis Wensley
Modified: 2013-09-18 07:59 UTC (History)
4 users (show)

See Also:
Tags:


Attachments
Reproduces the problem in MonoMac (21.69 KB, application/zip)
2011-09-27 16:32 UTC, Curtis Wensley
Details
Sample Objective-C application doing same thing (no crash) (37.42 KB, application/zip)
2011-09-27 16:33 UTC, Curtis Wensley
Details

Description Curtis Wensley 2011-09-27 16:32:41 UTC
Created attachment 517 [details]
Reproduces the problem in MonoMac

Currently, you cannot pass a custom NSObject-based class from NSTableViewDataSource.GetObjectValue, which is extremely important when creating custom NSCells.  This (most likely) has to do with how copyWithZone: is handled (or rather, that it is not?).

using Mono 2.10.6, MD 2.8b2, and MonoMac from git

The offending code is as simple as the following, using an NSTableView with a standard NSTextFieldCell:

Data source:

class MyDataSource : NSTableViewDataSource
{
	public override NSObject GetObjectValue (NSTableView tableView, NSTableColumn tableColumn, int row)
	{
		return new MyObject{ Text = (NSString)("My Row " + row) };
	}
}

Custom object:

public class MyObject : NSObject
{
	public MyObject () {}
		
	public MyObject (IntPtr handle) : base(handle) {}
		
	[Export("description")]
	public NSString Description {
		get { return Text; }
	}
	
	// using just a string here as a test, but will usually be something else
	public NSString Text { get; set; }
		
	[Export("copyWithZone:")]
	public NSObject CopyWithZone (IntPtr zone)
	{
		return new MyObject{
			Text = this.Text
		};
	}
}

Steps to reproduce:
1. Load the attached sample (TestMonoMacCustomCellData.zip)
2. Scroll around or resize the window

I've also created a sample app in Objective C to show how it works natively.
Comment 1 Curtis Wensley 2011-09-27 16:33:35 UTC
Created attachment 518 [details]
Sample Objective-C application doing same thing (no crash)
Comment 2 Curtis Wensley 2011-09-27 16:34:28 UTC
Here is the crash output:

Stacktrace:

  at (wrapper managed-to-native) MonoMac.ObjCRuntime.Messaging.void_objc_msgSendSuper (intptr,intptr) <0xffffffff>
  at MonoMac.Foundation.NSObject/MonoMac_Disposer.Drain (MonoMac.Foundation.NSObject) <0x00097>
  at (wrapper dynamic-method) object.[MonoMac.Foundation.NSObject+MonoMac_Disposer.Void Drain(MonoMac.Foundation.NSObject)] (MonoMac.Foundation.NSObject,MonoMac.ObjCRuntime.Selector,MonoMac.Foundation.NSObject) <0x00033>
  at (wrapper native-to-managed) object.[MonoMac.Foundation.NSObject+MonoMac_Disposer.Void Drain(MonoMac.Foundation.NSObject)] (MonoMac.Foundation.NSObject,MonoMac.ObjCRuntime.Selector,MonoMac.Foundation.NSObject) <0xffffffff>
  at (wrapper managed-to-native) MonoMac.AppKit.NSApplication.NSApplicationMain (int,string[]) <0xffffffff>
  at MonoMac.AppKit.NSApplication.Main (string[]) <0x00017>
  at TestMonoMacCustomCell.MainClass.Main (string[]) <0x00017>
  at (wrapper runtime-invoke) <Module>.runtime_invoke_void_object (object,intptr,intptr,intptr) <0xffffffff>

Native stacktrace:

	0   TestMonoMacCustomCell               0x000b90c6 mono_handle_native_sigsegv + 422
	1   TestMonoMacCustomCell               0x00003f3e mono_sigsegv_signal_handler + 334
	2   libsystem_c.dylib                   0x901c459b _sigtramp + 43
	3   ???                                 0xffffffff 0x0 + 4294967295
	4   ???                                 0x02709974 0x0 + 40933748
	5   ???                                 0x027116e8 0x0 + 40965864
	6   ???                                 0x020650dc 0x0 + 33968348
	7   ???                                 0x0204ed84 0x0 + 33877380
	8   CoreFoundation                      0x96c7c901 -[NSObject performSelector:withObject:] + 65
	9   Foundation                          0x90d105d4 __NSThreadPerformPerform + 503
	10  CoreFoundation                      0x96bf410f __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 15
	11  CoreFoundation                      0x96bf3ac6 __CFRunLoopDoSources0 + 246
	12  CoreFoundation                      0x96c1d9d8 __CFRunLoopRun + 1112
	13  CoreFoundation                      0x96c1d1ec CFRunLoopRunSpecific + 332
	14  CoreFoundation                      0x96c1d098 CFRunLoopRunInMode + 120
	15  HIToolbox                           0x99eb3487 RunCurrentEventLoopInMode + 318
	16  HIToolbox                           0x99ebacee ReceiveNextEventCommon + 168
	17  HIToolbox                           0x99ebac32 BlockUntilNextEventMatchingListInMode + 88
	18  AppKit                              0x9742b8ec _DPSNextEvent + 678
	19  AppKit                              0x9742b159 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 113
	20  AppKit                              0x974274cb -[NSApplication run] + 904
	21  AppKit                              0x976bab54 NSApplicationMain + 1054
	22  ???                                 0x027087f0 0x0 + 40929264
	23  ???                                 0x02708738 0x0 + 40929080
	24  ???                                 0x004bcda0 0x0 + 4967840
	25  ???                                 0x004bce37 0x0 + 4967991
	26  TestMonoMacCustomCell               0x0000fa34 mono_jit_runtime_invoke + 164
	27  TestMonoMacCustomCell               0x001be768 mono_runtime_invoke + 137
	28  TestMonoMacCustomCell               0x001c0f6a mono_runtime_exec_main + 669
	29  TestMonoMacCustomCell               0x001c02a4 mono_runtime_run_main + 843
	30  TestMonoMacCustomCell               0x000888cb mono_main + 7755
	31  TestMonoMacCustomCell               0x00001ed6 start + 54
Comment 3 Curtis Wensley 2011-09-27 19:15:01 UTC
Found a way to make things 'not crash', by adding a 'dealloc' method to clear out the IntPtr (which unregisters it)..  I wonder if this should be added to NSObject for all objects, in the case where the obj-c framework dealloc's an object directly?  It seems that MonoMac only unregisters objects via GC, but I could be missing something.

I was getting weird casting errors without this as well, as it'd use the same IntPtr for new objects

public class MyObject : NSObject
{
	bool nodispose;

	public MyObject ()
	{
	}
	
	public MyObject (IntPtr handle) : base(handle)
	{
	}
	
	[Export("description")]
	public NSString Description {
		get { return Text; }
	}
	
	public NSString Text { get; set; }
	
	[Export("copyWithZone:")]
	public NSObject CopyWithZone (IntPtr zone)
	{
		return new MyObject{
			Text = this.Text,
			nodispose = true
		};
	}
	
	[Export("dealloc")]
	public virtual void Dealloc()
	{
		if (nodispose)
			Handle = IntPtr.Zero;
	}
}
Comment 4 Martin Baulig 2012-08-20 03:53:53 UTC
Well, this is a ref-counting/ownership problem:

You create a new MyObject instance in your MyDataSource.GetObjectValue(), then return it to native code, without keeping a reference to it.  After returning, you do not own that object anymore, but the managed garbage collector does not know that.

Simply store the objects in a list, like this:

====
	List<MyObject> list;

	public MyDataSource ()
	{
		list = new List<MyObject> ();
		for (int i = 0; i < 10; i++) {
			list.Add (new MyObject { Text = "My Row " + i });
		}
	}

	public override NSObject GetObjectValue (NSTableView tableView, NSTableColumn tableColumn, int row)
	{
		return list [row];
	}

	public override int GetRowCount (NSTableView tableView)
	{
		return list.Count;
	}
====

However, this does not solve your copyWithZone: problem.  Here, storing the cloned objects locally is not an option, this would leak a lot of memory quickly.  Instead, you need to call retain on the cloned object.  Unfortunately, NSObject.Retain() is internal in MonoMac.dll, but you can just simply do it like this:

=====
	static IntPtr selRetain = Selector.GetHandle ("retain");
	[Export("copyWithZone:")]
	public NSObject CopyWithZone (IntPtr zone)
	{
		var cloned = new MyObject { Text = this.Text };
		Messaging.void_objc_msgSend (cloned.Handle, selRetain);
		return cloned;
	}
=====
Comment 5 Martin Baulig 2012-08-20 03:57:51 UTC
Btw. copyWithZone: is not called if you either return NSString in your NSTableViewDataSource.GetObjectValue() implementation or use NSTableViewDelegate.GetViewForItem().
Comment 6 Martin Baulig 2012-11-21 22:39:37 UTC
There has been some discussion about this on stackoverflow:
http://stackoverflow.com/questions/13216971/how-to-implement-cocoa-copywithzone-on-derived-object-in-monomac-c

We may want to provide some default / recommended way for how to copy an object and deal with the ref-counting issues - either by making CopyWithZone() public or providing some documenting.

Any ideas or suggestions ?
Comment 7 Rolf Bjarne Kvinge 2013-07-23 12:56:38 UTC
The CoreFoundation Memory Management Policy [1] states that Copy* methods must return a retained object. This means that the second part of Martin's fix in comment 4 is correct, we only need to add a public API to retain an object.

The first part of the fix in comment 4 should not be required for custom types, no custom objects are collected as long as native code has a reference to it. What might happen is that the object is collected before native code has a chance to retain it - and the fix for this would be to use retain+autorelease, so that it stays alive until the stack is unwound up to the first autorelease pool.

[1] https://developer.apple.com/library/mac/#documentation/CoreFoundation/Conceptual/CFMemoryMgmt/Concepts/Ownership.html
Comment 8 Rolf Bjarne Kvinge 2013-07-31 11:48:10 UTC
Fixed. NSObject will have three new methods: Retain, Release and Autorelease. The fix for this bug is to call Retain on the object returned from CopyWithZone:

    [Export("copyWithZone:")]
    public NSObject CopyWithZone (IntPtr zone)
    {
        return new MyObject{
            Text = this.Text,
            nodispose = true
        }.Retain ();
    }


xamcore: 365e4877aa49457a679cc05aa16d1b3b77a35f07
Comment 9 Nikolay Ivanets 2013-09-12 04:17:43 UTC
Any ETA when this fix to be available?
Comment 10 Rolf Bjarne Kvinge 2013-09-18 07:48:13 UTC
I haven't checked if the current stable already includes it, but if not it'll be included in the upcoming Xamarin.iOS 7.0 release
Comment 11 Nikolay Ivanets 2013-09-18 07:59:17 UTC
I asked about availability for Mac platform.

Note You need to log in before you can comment on or make changes to this bug.