Bug 10204 - NSTextView dodgy delegate situations
Summary: NSTextView dodgy delegate situations
Status: CLOSED FIXED
Alias: None
Product: Xamarin.Mac
Classification: Desktop
Component: Library (Xamarin.Mac.dll) ()
Version: 1.0.x
Hardware: PC Mac OS
: --- normal
Target Milestone: 2.0.x
Assignee: Chris Hamons
URL:
Depends on:
Blocks:
 
Reported: 2013-02-11 07:26 UTC by Jérémie Laval
Modified: 2015-03-26 16:55 UTC (History)
5 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:
CLOSED FIXED

Description Jérémie Laval 2013-02-11 07:26:04 UTC
Depending on the order you set events/delegates on an NSTextView, you might end up with incompatible Objective-C delegate types which make MonoMac blow up with InvalidCastException.

Works:

view.DidChangeSelection += ...;
view.TextDidEndEditing += ...;

Blows up:

view.TextDidEndEditing += ...;
view.DidChangeSelection += ...;

(Similar issue with other combinations)
Comment 1 Sebastien Pouliot 2013-02-11 12:07:12 UTC
It looks similar to [1] where using events differs from using a delegate (which is more atomic). The "working" order can even change between iOS (and I guess OSX) versions. Sadly the two rules from [1] can't cover cases where only settings events is causing problems.

[1] http://spouliot.wordpress.com/2012/03/26/events-vs-objective-c-delegates/
Comment 2 Jérémie Laval 2013-02-11 12:42:54 UTC
Also looking at the generated code, I think there might an extra issue with NSTextView.

The pattern for the Ensure*Delegate method is to do

var del = Delegate
if (del == null || !(del is MyClassSpecificDelegateImplementer))
    Delegate = new MyClassSpecificDelegateImplementer ();


Now the problem with NSTextView is that it inherits from NSText and the events/delegate are spread between these two classes.

Problem: each of them has its own MyClassDpecificDelegateImplementer which have no inheritance relationship between them which (I think) means that every time you access an event belonging to either side, the Delegate is recreated i.e. the events set from the other side are forgotten.
Comment 3 Jérémie Laval 2013-02-12 05:22:08 UTC
Seems like the crasher itself vanished with more recent version although maybe with the introduction of the problem in comment #2
Comment 4 Jérémie Laval 2013-02-22 05:01:19 UTC
I can reproduce the crasher in master with something like 

var view = new NSTextView () { Editable = true, FieldEditor = true };
view.TextDidChange += (sender, e) => ...;
view.TextDidEndEditing += (sender, e) => ...;
view.DidChangeSelection += (sender, e) => ...;


System.InvalidCastException: Cannot cast from source type to destination type.
  at MonoMac.AppKit.NSTextView.get_Delegate () [0x00012] in /Users/jeremie/mono/monodevelop/main/external/monomac/src/AppKit/NSTextView.g.cs:2825 
  at MonoMac.AppKit.NSTextView.EnsureNSTextViewDelegate () [0x00001] in /Users/jeremie/mono/monodevelop/main/external/monomac/src/AppKit/NSTextView.g.cs:3301 
  at MonoMac.AppKit.NSTextView.add_DidChangeSelection (System.EventHandler value) [0x00001] in /Users/jeremie/mono/monodevelop/main/external/monomac/src/AppKit/NSTextView.g.cs:3576
Comment 5 Jérémie Laval 2013-02-22 05:11:42 UTC
And as said in comment 2, reordering the declaration to something like:

var view = new NSTextView () { Editable = true, FieldEditor = true };
view.DidChangeSelection += (sender, e) => ...;
view.TextDidChange += (sender, e) => ...;
view.TextDidEndEditing += (sender, e) => ...;

Remove the crasher but the NSTextDelegate-based event (TextDidChange, TextDidEndEditing) erase the delegate thus remove DidChangeSelection (which is from NSTextViewDelegate).
Comment 6 Chris Hamons 2015-01-16 16:28:39 UTC
I'm working on this w/ my delegate rework for another bug, but this should be fixed there...
Comment 7 Chris Hamons 2015-02-18 12:09:48 UTC
Fixed in 2491f2f734e3052aabbfa9ac76c60ec8e23d7748 on master.
Comment 8 Chris Hamons 2015-02-18 12:38:06 UTC
Oh, and this is only fixed for the Unified Xamarin.Mac APi. The change was invasive / possibly breaking enough that we are making zero changes for Classic.
Comment 9 Rajneesh Kumar 2015-03-09 08:31:46 UTC
We are trying to verify this issue, but we are not sure about the steps to verify/Check this issue.

Could you please share steps to reproduce/check this issue? If possible please provide us a small sample, So that we can verify it efficiently.

Thanks