From 41e2339d99b2808d8b00de538afe5a1b294fda46 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Thu, 8 Aug 2024 09:12:47 +0200 Subject: [PATCH 1/2] [ObjCRuntime] Allow a null delegate in Runtime.ReleaseBlockWhenDelegateIsCollected. Fixes #20920. 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 | 6 +- tests/bindings-test/ApiDefinition.cs | 2 + tests/bindings-test/Messaging.cs | 3 + tests/bindings-test/RegistrarBindingTest.cs | 72 +++++++++++++++++---- tests/test-libraries/libtest.h | 9 +++ tests/test-libraries/libtest.m | 6 ++ 6 files changed, 83 insertions(+), 15 deletions(-) diff --git a/src/ObjCRuntime/Runtime.cs b/src/ObjCRuntime/Runtime.cs index a95371782f68..eab1b4046727 100644 --- a/src/ObjCRuntime/Runtime.cs +++ b/src/ObjCRuntime/Runtime.cs @@ -2587,12 +2587,12 @@ public static void ReleaseBlockOnMainThread (IntPtr block) [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 { diff --git a/tests/bindings-test/ApiDefinition.cs b/tests/bindings-test/ApiDefinition.cs index ddc40ac38a08..349eb550a6b3 100644 --- a/tests/bindings-test/ApiDefinition.cs +++ b/tests/bindings-test/ApiDefinition.cs @@ -424,6 +424,8 @@ interface ObjCBlockTester { [Export ("setProtocolWithBlockProperties:required:instance:")] void SetProtocolWithBlockProperties (IProtocolWithBlockProperties obj, bool required, bool instance); + [Export ("nullableCallback:")] + bool NullableCallback ([NullAllowed] Action completionHandler); } delegate void InnerBlock (int magic_number); diff --git a/tests/bindings-test/Messaging.cs b/tests/bindings-test/Messaging.cs index 737b00aeab86..bb6e8250ba7f 100644 --- a/tests/bindings-test/Messaging.cs +++ b/tests/bindings-test/Messaging.cs @@ -13,5 +13,8 @@ public struct objc_super { [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); } } diff --git a/tests/bindings-test/RegistrarBindingTest.cs b/tests/bindings-test/RegistrarBindingTest.cs index c2f2ba5eaa6c..63f49a442c1a 100644 --- a/tests/bindings-test/RegistrarBindingTest.cs +++ b/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 Bindings.Test; +#nullable enable + namespace Xamarin.BindingTests { [TestFixture] [Preserve (AllMembers = true)] @@ -102,6 +107,13 @@ public void DerivedClassBlockCallback () 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,55 @@ public override void ClassCallback (Action completionHandler) { completionHandler (42); } + + public override bool NullableCallback (Action? completionHandler) + { + if (completionHandler is not null) { + completionHandler (24); + return true; + } + return false; + } + +#if NET + public bool InvokeNullableCallbackNatively (Action? completionHandler) + { + byte rv; + if (completionHandler is null) { + rv = Messaging.byte_objc_msgSend_IntPtr (Handle, Selector.GetHandle ("nullableCallback:"), IntPtr.Zero); + } else { + unsafe { + delegate* unmanaged trampoline = &NullableCallbackHandler; + using var block = new BlockLiteral (trampoline, completionHandler, GetType (), nameof (NullableCallbackHandler)); + BlockLiteral* block_ptr = █ + 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> (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 +339,15 @@ public void ProtocolWithNativeBlockProperties (bool required, bool instance) 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 +401,16 @@ public void LinkedAway (bool required, bool instance) // 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; } } } } diff --git a/tests/test-libraries/libtest.h b/tests/test-libraries/libtest.h index e481dab7ec6d..219a0e59b7b8 100644 --- a/tests/test-libraries/libtest.h +++ b/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) 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 diff --git a/tests/test-libraries/libtest.m b/tests/test-libraries/libtest.m index 01dfb8bc991c..527778581999 100644 --- a/tests/test-libraries/libtest.m +++ b/tests/test-libraries/libtest.m @@ -812,6 +812,12 @@ +(int) calledBlockCount 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 From db78f3c527addec3108ecd11b6f12198d04f8267 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Thu, 8 Aug 2024 16:35:37 +0200 Subject: [PATCH 2/2] Make block code optimizable. --- tests/bindings-test/RegistrarBindingTest.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/bindings-test/RegistrarBindingTest.cs b/tests/bindings-test/RegistrarBindingTest.cs index 63f49a442c1a..94eaf396e667 100644 --- a/tests/bindings-test/RegistrarBindingTest.cs +++ b/tests/bindings-test/RegistrarBindingTest.cs @@ -257,6 +257,7 @@ public override bool NullableCallback (Action? completionHandler) } #if NET + [BindingImpl (BindingImplOptions.Optimizable)] public bool InvokeNullableCallbackNatively (Action? completionHandler) { byte rv; @@ -265,7 +266,7 @@ public bool InvokeNullableCallbackNatively (Action? completionHandler) } else { unsafe { delegate* unmanaged trampoline = &NullableCallbackHandler; - using var block = new BlockLiteral (trampoline, completionHandler, GetType (), nameof (NullableCallbackHandler)); + using var block = new BlockLiteral (trampoline, completionHandler, typeof (BlockCallbackTester), nameof (NullableCallbackHandler)); BlockLiteral* block_ptr = █ rv = Messaging.byte_objc_msgSend_IntPtr (Handle, Selector.GetHandle ("nullableCallback:"), (IntPtr) block_ptr); }