Bug 8585 - StillMotion sample crashes on NSNotification
Summary: StillMotion sample crashes on NSNotification
Status: RESOLVED FIXED
Alias: None
Product: MonoMac
Classification: Desktop
Component: Bindings ()
Version: GIT
Hardware: Macintosh Mac OS
: --- normal
Target Milestone: ---
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2012-11-24 16:45 UTC by Aaron Oneal
Modified: 2015-02-11 20:16 UTC (History)
4 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 FIXED

Description Aaron Oneal 2012-11-24 16:45:04 UTC
The StillMotion sample is crashing immediately after receiving an NSNotification. It doesn't happen right away, but move the window around and task switch a few times and this should repro in a minute or two.

Stacktrace:

  at (wrapper managed-to-native) MonoMac.AppKit.NSApplication.NSApplicationMain (int,string[]) <IL 0x0009d, 0xffffffff>
  at MonoMac.AppKit.NSApplication.Main (string[]) [0x0000a] in AppKit/NSApplication.cs:77
  at StillMotion.MainClass.Main (string[]) [0x00005] in monomac/samples/StillMotion/Main.cs:14
  at (wrapper runtime-invoke) <Module>.runtime_invoke_void_object (object,intptr,intptr,intptr) <IL 0x00050, 0xffffffff>

Native stacktrace:

	0   libmono-2.0.dylib                   0x001c6d6f mono_handle_native_sigsegv + 287
	1   libmono-2.0.dylib                   0x0012bf1e mono_sigsegv_signal_handler + 334
	2   libsystem_c.dylib                   0x976fd86b _sigtramp + 43
	3   ???                                 0xffffffff 0x0 + 4294967295
	4   CoreFoundation                      0x99fdee01 ___CFXNotificationPost_block_invoke_0 + 257
	5   CoreFoundation                      0x99f2a43a _CFXNotificationPost + 2794
	6   Foundation                          0x97c8c788 -[NSNotificationCenter postNotificationName:object:userInfo:] + 92
	7   Foundation                          0x97c9c527 -[NSNotificationCenter postNotificationName:object:] + 55
	8   AppKit                              0x94268fd8 -[NSWindow update] + 74
	9   libobjc.A.dylib                     0x900d6586 -[NSObject performSelector:] + 62
	10  CoreFoundation                      0x99f77d7d -[NSArray makeObjectsPerformSelector:] + 333
	11  AppKit                              0x941ad6c8 -[NSApplication(NSWindowCache) _updateWindowsUsingCache] + 449
	12  AppKit                              0x941ad47e -[NSApplication updateWindows] + 93
	13  AppKit                              0x944011da __38-[NSApplication setWindowsNeedUpdate:]_block_invoke_02428 + 73
	14  CoreFoundation                      0x99f6e28d _runLoopObserverWithBlockContext + 29
	15  CoreFoundation                      0x99f3edfe __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 30
	16  CoreFoundation                      0x99f3ed3d __CFRunLoopDoObservers + 381
	17  CoreFoundation                      0x99f18e16 __CFRunLoopRun + 886
	18  CoreFoundation                      0x99f1863a CFRunLoopRunSpecific + 378
	19  CoreFoundation                      0x99f184ab CFRunLoopRunInMode + 123
	20  HIToolbox                           0x97fb615a RunCurrentEventLoopInMode + 242
	21  HIToolbox                           0x97fb5df5 ReceiveNextEventCommon + 162
	22  HIToolbox                           0x97fb5d44 BlockUntilNextEventMatchingListInMode + 88
	23  AppKit                              0x941aba3a _DPSNextEvent + 724
	24  AppKit                              0x941ab26c -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 119
	25  AppKit                              0x941a16cc -[NSApplication run] + 855
	26  AppKit                              0x941446f6 NSApplicationMain + 1053
	27  ???                                 0x03372f76 0x0 + 53948278
	28  ???                                 0x03372c9c 0x0 + 53947548
	29  ???                                 0x00927ff8 0x0 + 9601016
	30  ???                                 0x00928156 0x0 + 9601366
	31  libmono-2.0.dylib                   0x00135ca4 mono_jit_runtime_invoke + 164
	32  libmono-2.0.dylib                   0x002ab2e4 mono_runtime_invoke + 68
	33  libmono-2.0.dylib                   0x002b13ae mono_runtime_exec_main + 238
	34  libmono-2.0.dylib                   0x0019b5fd mono_main + 6797
	35  StillMotion                         0x000cf4bf main + 3135
	36  StillMotion                         0x000ce875 start + 53
Comment 1 Aaron Oneal 2012-11-24 17:10:11 UTC
Here's another trace. This one seems to be in response to sending windowDidUpdate to an NSNumber. Maybe a handle is resolving incorrectly somewhere?

2012-11-24 14:04:54.880 StillMotion[22850:1007] -[__NSCFNumber windowDidUpdate:]: unrecognized selector sent to instance 0x7c5daf50
2012-11-24 14:04:54.881 StillMotion[22850:1007] (
	0   CoreFoundation                      0x9a01d12b __raiseError + 219
	1   libobjc.A.dylib                     0x900c952e objc_exception_throw + 230
	2   CoreFoundation                      0x9a020d9d -[NSObject(NSObject) doesNotRecognizeSelector:] + 253
	3   CoreFoundation                      0x99f69437 ___forwarding___ + 487
	4   CoreFoundation                      0x99f691e2 _CF_forwarding_prep_0 + 50
	5   Foundation                          0x97ca3c52 __57-[NSNotificationCenter addObserver:selector:name:object:]_block_invoke_0 + 49
	6   CoreFoundation                      0x99fdee01 ___CFXNotificationPost_block_invoke_0 + 257
	7   CoreFoundation                      0x99f2a43a _CFXNotificationPost + 2794
	8   Foundation                          0x97c8c788 -[NSNotificationCenter postNotificationName:object:userInfo:] + 92
	9   Foundation                          0x97c9c527 -[NSNotificationCenter postNotificationName:object:] + 55
	10  AppKit                              0x94268fd8 -[NSWindow update] + 74
	11  libobjc.A.dylib                     0x900d6586 -[NSObject performSelector:] + 62
	12  CoreFoundation                      0x99f77d7d -[NSArray makeObjectsPerformSelector:] + 333
	13  AppKit                              0x941ad6c8 -[NSApplication(NSWindowCache) _updateWindowsUsingCache] + 449
	14  AppKit                              0x941ad47e -[NSApplication updateWindows] + 93
	15  AppKit                              0x944011da __38-[NSApplication setWindowsNeedUpdate:]_block_invoke_02428 + 73
	16  CoreFoundation                      0x99f6e28d _runLoopObserverWithBlockContext + 29
	17  CoreFoundation                      0x99f3edfe __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 30
	18  CoreFoundation                      0x99f3ed3d __CFRunLoopDoObservers + 381
	19  CoreFoundation                      0x99f18e16 __CFRunLoopRun + 886
	20  CoreFoundation                      0x99f1863a CFRunLoopRunSpecific + 378
	21  CoreFoundation                      0x99f184ab CFRunLoopRunInMode + 123
	22  HIToolbox                           0x97fb615a RunCurrentEventLoopInMode + 242
	23  HIToolbox                           0x97fb5df5 ReceiveNextEventCommon + 162
	24  HIToolbox                           0x97fb5d44 BlockUntilNextEventMatchingListInMode + 88
	25  AppKit                              0x941aba3a _DPSNextEvent + 724
	26  AppKit                              0x941ab26c -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 119
	27  AppKit                              0x941a16cc -[NSApplication run] + 855
	28  AppKit                              0x941446f6 NSApplicationMain + 1053
	29  ???                                 0x032c3f76 0x0 + 53231478
	30  ???                                 0x032c3c9c 0x0 + 53230748
	31  ???                                 0x0087cff8 0x0 + 8900600
	32  ???                                 0x0087d156 0x0 + 8900950
	33  libmono-2.0.dylib                   0x0008aca4 mono_jit_runtime_invoke + 164
	34  libmono-2.0.dylib                   0x002002e4 mono_runtime_invoke + 68
	35  libmono-2.0.dylib                   0x002063ae mono_runtime_exec_main + 238
	36  libmono-2.0.dylib                   0x000f05fd mono_main + 6797
	37  StillMotion                         0x000244bf main + 3135
	38  StillMotion                         0x00023875 start + 53
)
Comment 2 Aaron Oneal 2012-11-25 15:16:01 UTC
After adding some extensive logging, I think I've found a couple problems with memory management.

First, Retain() is being called by NativeConstructorBuilder after a managed/custom NSObject is constructed. I don't know why the code is doing this because near as I can tell the managed object was just alloced thanks to the NSObject() base class and owns the alloc, so I don't believe a Retain() is necessary. In fact, this was probably hiding some things and leaking by adding an extra ref count. 

I recommend removing this call to Retain() unless there is a specific reason for it I've overlooked.

Second, in this sample the automatic delegates that get created by the generated EnsureXYZDelegate() calls are getting finalized by the GC triggering a Dispose(false), but in that Dispose() the handle is Unregistered from the Runtime. At some point a notification comes in and the Handle for that delegate is no longer found because it was removed by the finalizer. GetNSObject then tries to activate a new instance with the IntPtr that just came in which fails because there is no IntPtr constructor on those auto-delegate types (nor should there be as they're fully managed maintained).

So why is the delegate getting disposed? Because the window is being disposed. The only reference on the managed side to the window in the sample is created in:

public override void WindowControllerDidLoadNib (NSWindowController windowController)

NSWindowController.Window creates a managed instance via IntPtr, the delegate is created, and the function goes on to register a bunch of events. But, nowhere is a reference to the controller or window kept on the managed side, so by the end of the method, the GC collects it and disposes of the delegates which then cause the notification failures above.

This happens in other NSDocument samples too like CoreTextArcMonoMac.

I'm not sure yet what the right fix is. I mean, it's easy enough to have the sample keep a reference to the window controller sent by this method and this solves the issue. But, for one thing, it was a royal pain to debug, and as was the case with the sample writer, one would not expect to have to cache a reference on the managed side for this. 
- Should the base class instead keep a reference to the controller so app developers don't need to? 
- Are there other cases in other types where this could occur and is it only isolated to auto-delegate scenarios?
- Can we make GetNSObject smarter so that if an IntPtr comes in for one of these delegates we give a more helpful exception (e.g. you forgot to keep a managed reference) rather than crashing or failing to construct one of these built-in delegates that shouldn't be getting constructed in the first place?

Finally, I came across a couple types that have an internal ctor(IntPtr), but GetNSObject is currently relying on Activator which doesn't support internal. Either those types should change or the code should use reflection here to invoke the constructor directly.

I'm happy to submit a patch for some of these changes but would appreciate feedback on direction first. Thanks!
Comment 3 Aaron Oneal 2012-11-25 15:35:38 UTC
Looks like the constructor Retain() results in a callback to [retain:] which invokes NativeRetain() which then allocs a gchandle to keep the object from going away when there is a native only reference. So, scratch that part, looks like the Retain() is required, but the rest of the issues above still need to be addressed.
Comment 4 Aaron Oneal 2012-11-25 17:51:25 UTC
Scratch the Activator part too, I was looking at some INativeObjects.

Regarding keeping the reference around to NSWindow(Controller), which is the main issue causing this bug, I see the KeepRefUntil member was added for a similar situation, but at the moment I don't see a great way to apply it to this case.
Comment 5 Miguel de Icaza [MSFT] 2012-12-08 21:56:34 UTC
 The problem here is that a native object is being surfaced temporarily, purely for the sake of running the callback, but nothing keeps a reference to it from managed code.    

For (c) what I suspect we need to do is hijack the retain/release methods for the class in question, and treat these surfaced objects to the same GCHandle treatment that we give to our code, for as long as there is a reference held from unmanaged code.

We are going to have a discussion about this pattern this week and think about some options here.
Comment 6 Timothy Risi 2015-02-11 20:16:57 UTC
I tested this on the current stable builds and have been unable to reproduce the crash so I'm going to close the issue as fixed.  It can be reopened if it turns out to still be an issue.