[ObjCRuntime] Allow a null delegate in Runtime.ReleaseBlockWhenDelegateIsCollected. Fixes #20920. (#20999)

Allow a null delegate in Runtime.ReleaseBlockWhenDelegateIsCollected if the corresponding native pointer is also null.

Fixes https://github.com/xamarin/xamarin-macios/issues/20920.

src/ObjCRuntime/Runtime.cs

@@ -2587,12 +2587,12 @@ namespace ObjCRuntime {
 		[EditorBrowsable (EditorBrowsableState.Never)]
 		static Delegate ReleaseBlockWhenDelegateIsCollected (IntPtr block, Delegate @delegate)
 		{
-			if (@delegate is null)
-				ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (@delegate));
-
 			if (block == IntPtr.Zero)
 				return @delegate;
 
+			if (@delegate is null)
+				ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (@delegate));
+
 			if (block_lifetime_table.TryGetValue (@delegate, out var existingCollector)) {
 				existingCollector.Add (block);
 			} else {

tests/bindings-test/ApiDefinition.cs

@@ -424,6 +424,8 @@ namespace Bindings.Test {
 		[Export ("setProtocolWithBlockProperties:required:instance:")]
 		void SetProtocolWithBlockProperties (IProtocolWithBlockProperties obj, bool required, bool instance);
 
+		[Export ("nullableCallback:")]
+		bool NullableCallback ([NullAllowed] Action<int> completionHandler);
 	}
 
 	delegate void InnerBlock (int magic_number);

tests/bindings-test/Messaging.cs

@@ -13,5 +13,8 @@ namespace ObjCRuntime {
 
 		[DllImport (LIBOBJC_DYLIB, EntryPoint = "objc_msgSend")]
 		public extern static void void_objc_msgSend_IntPtr_bool_bool (IntPtr receiver, IntPtr selector, IntPtr a, bool b, bool c);
+
+		[DllImport (LIBOBJC_DYLIB, EntryPoint = "objc_msgSend")]
+		public extern static byte byte_objc_msgSend_IntPtr (IntPtr receiver, IntPtr selector, IntPtr a);
 	}
 }

tests/bindings-test/RegistrarBindingTest.cs

@@ -1,5 +1,8 @@
 using System;
 using System.Threading;
+#if NET
+using System.Runtime.InteropServices;
+#endif
 
 using Foundation;
 using ObjCRuntime;
@@ -8,6 +11,8 @@ using NUnit.Framework;
 
 using Bindings.Test;
 
+#nullable enable
+
 namespace Xamarin.BindingTests {
 	[TestFixture]
 	[Preserve (AllMembers = true)]
@@ -102,6 +107,13 @@ namespace Xamarin.BindingTests {
 				ObjCBlockTester.CallRequiredStaticCallback ();
 				ObjCBlockTester.CallOptionalStaticCallback ();
 				DerivedBlockCallbackClass.Answer = 2;
+
+#if NET
+				Assert.IsFalse (obj.InvokeNullableCallbackNatively (null), "NullableCallback A rv");
+				int nullableResult = -1;
+				Assert.IsTrue (obj.InvokeNullableCallbackNatively ((v) => nullableResult = v), "NullableCallback B rv");
+				Assert.AreEqual (24, nullableResult, "NullableCallback result");
+#endif
 			}
 		}
 
@@ -234,19 +246,56 @@ namespace Xamarin.BindingTests {
 			{
 				completionHandler (42);
 			}
+
+			public override bool NullableCallback (Action<int>? completionHandler)
+			{
+				if (completionHandler is not null) {
+					completionHandler (24);
+					return true;
+				}
+				return false;
+			}
+
+#if NET
+			[BindingImpl (BindingImplOptions.Optimizable)]
+			public bool InvokeNullableCallbackNatively (Action<int>? completionHandler)
+			{
+				byte rv;
+				if (completionHandler is null) {
+					rv = Messaging.byte_objc_msgSend_IntPtr (Handle, Selector.GetHandle ("nullableCallback:"), IntPtr.Zero);
+				} else {
+					unsafe {
+						delegate* unmanaged<IntPtr, int, void> trampoline = &NullableCallbackHandler;
+						using var block = new BlockLiteral (trampoline, completionHandler, typeof (BlockCallbackTester), nameof (NullableCallbackHandler));
+						BlockLiteral* block_ptr = &block;
+						rv = Messaging.byte_objc_msgSend_IntPtr (Handle, Selector.GetHandle ("nullableCallback:"), (IntPtr) block_ptr);
+					}
+				}
+				return rv != 0;
+			}
+
+			[UnmanagedCallersOnly]
+			static void NullableCallbackHandler (IntPtr block, int magicNumber)
+			{
+				var del = BlockLiteral.GetTarget<Action<int>> (block);
+				if (del is not null) {
+					del (magicNumber);
+				}
+			}
+#endif
 		}
 
 		public class PropertyBlock : NSObject, IProtocolWithBlockProperties {
 			[Export ("myOptionalProperty")]
-			public SimpleCallback MyOptionalProperty { get; set; }
+			public SimpleCallback MyOptionalProperty { get; set; } = () => { Assert.Fail ("No block set?"); };
 
-			public SimpleCallback MyRequiredProperty { get; set; }
+			public SimpleCallback MyRequiredProperty { get; set; } = () => { Assert.Fail ("No block set?"); };
 
 			[Export ("myOptionalStaticProperty")]
-			public static SimpleCallback MyOptionalStaticProperty { get; set; }
+			public static SimpleCallback? MyOptionalStaticProperty { get; set; }
 
 			[Export ("myRequiredStaticProperty")]
-			public static SimpleCallback MyRequiredStaticProperty { get; set; }
+			public static SimpleCallback? MyRequiredStaticProperty { get; set; }
 		}
 
 		[Test]
@@ -291,15 +340,15 @@ namespace Xamarin.BindingTests {
 				ObjCBlockTester.SetProtocolWithBlockProperties (pb, required, instance);
 				if (required) {
 					if (instance) {
-						pb.MyRequiredProperty ();
+						pb.MyRequiredProperty! ();
 					} else {
-						PropertyBlock.MyRequiredStaticProperty ();
+						PropertyBlock.MyRequiredStaticProperty! ();
 					}
 				} else {
 					if (instance) {
-						pb.MyOptionalProperty ();
+						pb.MyOptionalProperty! ();
 					} else {
-						PropertyBlock.MyOptionalStaticProperty ();
+						PropertyBlock.MyOptionalStaticProperty! ();
 					}
 				}
 				Assert.AreEqual (calledCounter + 1, ObjCBlockTester.CalledBlockCount, "Blocks called");
@@ -353,16 +402,16 @@ namespace Xamarin.BindingTests {
 		// have been linked away (which happen when _forcing_ the dynamic registrar to be linked away).
 		public class FakePropertyBlock : NSObject {
 			[Export ("myOptionalProperty")]
-			public SimpleCallback MyOptionalProperty { get; set; }
+			public SimpleCallback? MyOptionalProperty { get; set; }
 
 			[Export ("myRequiredProperty")]
-			public SimpleCallback MyRequiredProperty { get; set; }
+			public SimpleCallback? MyRequiredProperty { get; set; }
 
 			[Export ("myOptionalStaticProperty")]
-			public static SimpleCallback MyOptionalStaticProperty { get; set; }
+			public static SimpleCallback? MyOptionalStaticProperty { get; set; }
 
 			[Export ("myRequiredStaticProperty")]
-			public static SimpleCallback MyRequiredStaticProperty { get; set; }
+			public static SimpleCallback? MyRequiredStaticProperty { get; set; }
 		}
 	}
 }

tests/test-libraries/libtest.h

@@ -17,6 +17,10 @@ extern "C" {
 int theUltimateAnswer ();
 void useZLib ();
 
+// NS_ASSUME_NONNULL_BEGIN doesn't work
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wnullability-completeness"
+
 typedef void (^x_block_callback)();
 void x_call_block (x_block_callback block);
 void* x_call_func_3 (void* (*fptr)(void*, void*, void*), void* p1, void* p2, void* p3);
@@ -237,6 +241,8 @@ typedef void (^outerBlock) (innerBlock callback);
 
 +(void) setProtocolWithBlockProperties: (id<ProtocolWithBlockProperties>) obj required: (bool) required instance: (bool) instance;
 +(int) calledBlockCount;
+
+-(bool) nullableCallback: (void (^ __nullable)(int32_t magic_number))completionHandler;
 @end
 
 @interface FreedNotifier : NSObject {
@@ -294,6 +300,9 @@ typedef void (^outerBlock) (innerBlock callback);
 
 @end
 
+#pragma clang diagnostic pop
+// NS_ASSUME_NONNULL_END
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif

tests/test-libraries/libtest.m

@@ -812,6 +812,12 @@ static Class _TestClass = NULL;
 	return called_blocks;
 }
 
+-(bool) nullableCallback: (void (^ __nullable)(int32_t magic_number))completionHandler
+{
+	assert (false); // THIS FUNCTION SHOULD BE OVERRIDDEN
+	return false;
+}
+
 static void block_called ()
 {
 #pragma clang diagnostic push