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)

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


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 [MSFT] 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 [MSFT] 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 [MSFT] 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.

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.


Create a new report for Bug 1086 on Developer Community or GitHub if you have new information to add and do not yet see a matching report.

  • Export the original title and description: Developer Community HTML or GitHub Markdown
  • Copy the title and description into the new report. Adjust them to be up-to-date if needed.
  • Add your new information.

In special cases on GitHub you might also want the comments: GitHub Markdown with public comments


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.

Related Links: