From bcf3899893c0ee1a0bbcd814164a08423f0d543a Mon Sep 17 00:00:00 2001 From: monojenkins Date: Tue, 26 Nov 2019 19:38:58 +0100 Subject: [PATCH] [generator] If forcing a managed type, we must do so even if there's another managed instance of an incompatible type. Fixes #7441. (#7460) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If forcing a managed type for a particular native handle, we must ensure we're always successful. This means creating another managed instance even if there already is an existing instance for the native handle. This is not optimal, but the alternative is worse: some functionality can be completely broken otherwise (such as NSSavePanel⁄NSOpenPanel may stop working). If creating multiple managed instances for the same native handle ends up being too problematic, we'll probably have to add full support for this scenario (see #7442). Fixes https://github.com/xamarin/xamarin-macios/issues/7441. --- src/ObjCRuntime/Runtime.cs | 12 ++++++++++-- src/generator.cs | 6 +++--- .../monotouch-test/ObjCRuntime/RuntimeTest.cs | 19 +++++++++++++++++++ 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/ObjCRuntime/Runtime.cs b/src/ObjCRuntime/Runtime.cs index 17fdd4fa3f..8bd0ab9110 100644 --- a/src/ObjCRuntime/Runtime.cs +++ b/src/ObjCRuntime/Runtime.cs @@ -1422,6 +1422,11 @@ namespace ObjCRuntime { // this method is identical in behavior to the non-generic one. public static T GetINativeObject (IntPtr ptr, bool owns) where T : class, INativeObject + { + return GetINativeObject (ptr, false, owns); + } + + public static T GetINativeObject (IntPtr ptr, bool forced_type, bool owns) where T : class, INativeObject { if (ptr == IntPtr.Zero) return null; @@ -1433,7 +1438,10 @@ namespace ObjCRuntime { return t; } - if (o != null) { + // If forced type is true, we ignore any existing instances if the managed type of the existing instance isn't compatible with T. + // This may end up creating multiple managed wrapper instances for the same native handle, + // which is not optimal, but sometimes the alternative can be worse :/ + if (o != null && !forced_type) { // found an existing object, but with an incompatible type. if (!typeof (T).IsInterface && typeof(NSObject).IsAssignableFrom (typeof (T))) { // if the target type is another NSObject subclass, there's nothing we can do. @@ -1445,7 +1453,7 @@ namespace ObjCRuntime { var implementation = LookupINativeObjectImplementation (ptr, typeof (T)); if (implementation.IsSubclassOf (typeof (NSObject))) { - if (o != null) { + if (o != null && !forced_type) { // We already have an instance of an NSObject-subclass for this ptr. // Creating another will break the one-to-one assumption we have between // native objects and NSObject instances. diff --git a/src/generator.cs b/src/generator.cs index c665781dce..9b8eed3e1b 100644 --- a/src/generator.cs +++ b/src/generator.cs @@ -1649,7 +1649,7 @@ public partial class Generator : IMemberGatherer { if (IsProtocolInterface (pi.ParameterType)) { invoke.AppendFormat (" Runtime.GetINativeObject<{1}> ({0}, false)", pi.Name.GetSafeParamName (), pi.ParameterType); } else if (isForced) { - invoke.AppendFormat (" Runtime.GetINativeObject<{1}> ({0}, {2})", pi.Name.GetSafeParamName (), RenderType (pi.ParameterType), isForcedOwns); + invoke.AppendFormat (" Runtime.GetINativeObject<{1}> ({0}, true, {2})", pi.Name.GetSafeParamName (), RenderType (pi.ParameterType), isForcedOwns); } else { invoke.AppendFormat (" Runtime.GetNSObject<{1}> ({0})", pi.Name.GetSafeParamName (), RenderType (pi.ParameterType)); } @@ -3611,7 +3611,7 @@ public partial class Generator : IMemberGatherer { cast_b = ", false)"; } else if (minfo != null && minfo.is_forced) { cast_a = " Runtime.GetINativeObject<" + FormatType (declaringType, GetCorrectGenericType (mi.ReturnType)) + "> ("; - cast_b = $", {minfo.is_forced_owns})"; + cast_b = $", true, {minfo.is_forced_owns})"; } else if (minfo != null && minfo.is_bindAs) { var bindAs = GetBindAsAttribute (minfo.mi); var nullableBindAsType = TypeManager.GetUnderlyingNullableType (bindAs.Type); @@ -4138,7 +4138,7 @@ public partial class Generator : IMemberGatherer { } else if (isINativeObjectSubclass) { if (!pi.IsOut) by_ref_processing.AppendFormat ("if ({0}Value != ({0} == null ? IntPtr.Zero : {0}.Handle))\n\t", pi.Name.GetSafeParamName ()); - by_ref_processing.AppendFormat ("{0} = Runtime.GetINativeObject<{1}> ({0}Value, {2});\n", pi.Name.GetSafeParamName (), RenderType (elementType), isForcedType ? isForcedOwns : "false"); + by_ref_processing.AppendFormat ("{0} = Runtime.GetINativeObject<{1}> ({0}Value, {2}, {3});\n", pi.Name.GetSafeParamName (), RenderType (elementType), isForcedType ? "true" : "false", isForcedType ? isForcedOwns : "false"); } else { throw ErrorHelper.CreateError (99, $"Internal error: don't know how to create ref/out (output) code for {mai.Type} in {mi}. Please file a bug report with a test case (https://github.com/xamarin/xamarin-macios/issues/new)."); } diff --git a/tests/monotouch-test/ObjCRuntime/RuntimeTest.cs b/tests/monotouch-test/ObjCRuntime/RuntimeTest.cs index 124a938693..d6193148bd 100644 --- a/tests/monotouch-test/ObjCRuntime/RuntimeTest.cs +++ b/tests/monotouch-test/ObjCRuntime/RuntimeTest.cs @@ -699,5 +699,24 @@ Additional information: } #endif } + + [Test] + public void GetINativeObject_ForcedType () + { + using (var str = new NSString ("hello world")) { + NSDate date; + + Assert.Throws (() => Runtime.GetINativeObject (str.Handle, false), "EX1"); // + Assert.Throws (() => Runtime.GetINativeObject (str.Handle, false, false), "EX2"); // + + // Making a string quack like a date. + // This is a big hack, but hopefully it works well enough for this particular test case. + // Just don't inspect the date variable in a debugger (it will most likely crash the app). + date = Runtime.GetINativeObject (str.Handle, true, false); + Assert.AreEqual (date.Handle, str.Handle, "Same native pointer"); + date.Dispose (); + date = null; + } + } } }