Bug 10876 - Handling IntPtr.Zero from Selector return values (UIBarButtonItem.Action is not bound correctly)
Summary: Handling IntPtr.Zero from Selector return values (UIBarButtonItem.Action is n...
Status: RESOLVED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: Xamarin.iOS.dll ()
Version: 6.2.x
Hardware: PC Mac OS
: Normal normal
Target Milestone: Untriaged
Assignee: Miguel de Icaza [MSFT]
URL:
Depends on:
Blocks:
 
Reported: 2013-03-04 10:05 UTC by Alan McGovern
Modified: 2013-06-04 10:31 UTC (History)
3 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 Alan McGovern 2013-03-04 10:05:59 UTC
Invoking this property gives this exception:

System.ArgumentException: sel is not a selector handle.
Comment 1 Sebastien Pouliot 2013-03-04 11:45:37 UTC
UIBarItem does not have an Action property. From your earlier screenshot I think you're talking about UIBarButtonItem (which inherits from UIBarItem).
Comment 2 Sebastien Pouliot 2013-03-04 11:54:07 UTC
The exception occurs because it returns null, by default, and the generator tries to create a Selector on a null (IntPtr.Zero) handle - which is invalid (unusable), hence the exception.

The generator should do the right thing if the [NullAllowed] attribute was present (it's missing).
Comment 3 Sebastien Pouliot 2013-03-04 13:51:42 UTC
The exception is not related to the missing [NullAllowed] attribute (that was a binding bug - but unrelated).

The generated code does not handle a null selector (it cant return null - even if allowed).
Comment 4 Sebastien Pouliot 2013-03-04 15:47:41 UTC
The missing [NullAllowed] attribute fixed in master: 
ed13cb8fa4b62419298acb475c98aa4b55e928b7

The clean way to handle this (so no exception occurs when accessing the property) would be for the generator to return `null` when it gets a IntPtr.Zero value.

However that's a big change since the same pattern is used for almost every non-NSObject types bound (see method Go in generator.cs). Changing it everywhere could break existing code, since the instances we return are "somewhat" usable - at least as a null native object.

A specific (to Selector) fix could be to change "new Selector (" for a "Selector.Create (" and let that new static method return null if the handle is invalid (not just IntPtr.Zero).

Note that we do not see those exceptions often because most of the constructors do not validate the IntPtr parameter [1], like Selector does [2], so we end up with valid .NET instance that get used as `null` native objects in bindings.

[1] the recent change to throw on null/IntPtr.Zero Handle *only* applies to NSObject (not all other INativeObject ctor code);

[2] https://github.com/xamarin/monotouch/blob/master/src/ObjCRuntime/Selector.cs#L21

c.c. miguel
Comment 5 Miguel de Icaza [MSFT] 2013-03-11 12:43:28 UTC
I will investigate what we should do here for the generator to turn:

Selector SomeProperty { get { return new Selector (NativeMethods.objc_sendMsg (handle, "someProperty:"); } }

Into something that checks if the return is null, and if so return null.

The investigation is whether we should make the change or not.
Comment 6 Sebastien Pouliot 2013-03-11 16:26:17 UTC
My first idea was to do a _small_ change to generator.cs, e.g.

"new Selector" to "Selector.FromHandle ("

and create the later static method. But that would be a fix specific to Selector and leave it different from other NSObject (optionally throw) and other INativeObject (most oif them create empty shell instances).

OTOH the current behavior shows an exception inside XS watches while debugging applications - where people would expect null (since that's the default documented value).
Comment 7 Miguel de Icaza [MSFT] 2013-03-21 17:49:56 UTC
Let us proceed with the suggestion Sebastien for Selectors.
Comment 8 Sebastien Pouliot 2013-06-04 10:31:33 UTC
generator fixed in maccore: a7b5435b63f7cdc7380356ca5619d2d1d9eecd9f
new API added in ios: aebf3a25c136d07bd693e5d7e6dcc93837fda257
new API added in monomac: 8cd28352c2b31e9f6104e6d24234922a21a69a66